-
Notifications
You must be signed in to change notification settings - Fork 127
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
Fix possible race condition #702
base: master
Are you sure you want to change the base?
Fix possible race condition #702
Conversation
Did you get any deadlock or race conditions in practice? This structure is used in the context of one connection and processed only by two goroutines working sequentially. Due to sequential processing and no shared data between several connections, we don't use extra locks. We have next flow: Due to database protocols being synchronous and sequential (except extra cases of notifications that out of scope of processing by Acra), at first we send request, and then we get the response. So, two goroutines that handle client & db connections work sequentially and wouldn't do anything simultaneously due to protocols that don't support it. So, we don't need here any mutes that will only slow down processing |
I don't know that the "list" is thread safe by design, but in my test, i got this panic
|
Let's say your design is like that, maybe there is another issue in the implementation. Because i don't know the design, i just put mutex there |
thanks for the traceback. can you tell which tool or language + library you used for PostgreSQL? |
I use this library npgsql |
Fix possible race conditions.
Since container/list is not thread safe, so we need to lock the list during Add and Remove operations.
Checklist
with new changes