-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nested serialize.Executor.AsyncExec calls may deadlock when overwhelmed #512
Comments
Tested locally it is reproducible. |
To fix the issue we can use a concurrent queue OR change channelSize to the max count of request per one moment for GRPC. Fix via list + lock:
|
I've found additional deadlock with executor:
It looks like we need t rework Thoughts? |
I don't think the issue in this one is calling AsyncExec inside AsyncExec ... its blocking on the nested AsyncExec call. And yes, that's one probably best addressed by documentation. |
…etworkservicemesh#540) * use remote serialize executor to solve issue 512 Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> * make format Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> Signed-off-by: Sergey Ershov <sergey.ershov@xored.com>
…etworkservicemesh#540) * use remote serialize executor to solve issue 512 Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> * make format Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> Signed-off-by: Sergey Ershov <sergey.ershov@xored.com>
Example of code that deadlocks:
The issue is that
AsyncExec()
may block on channel write.We have code that uses nested
AsyncExec()
and thus may potentially deadlock:sdk/pkg/networkservice/connectioncontext/dnscontext/client.go
Line 107 in cf9c225
We should either document this limitation and rewrite the code that uses nested
AsyncExec()
OR reworkserialize.Executor
to support unlimited nested calls.The text was updated successfully, but these errors were encountered: