Add synchronization to prevent race in broker #14
Conversation
NullHypothesis
reviewed
Nov 27, 2019
broker/broker.go
Outdated
| @@ -37,6 +38,8 @@ type BrokerContext struct { | |||
| // Map keeping track of snowflakeIDs required to match SDP answers from | |||
| // the second http POST. | |||
| idToSnowflake map[string]*Snowflake | |||
| // Synchronization for the | |||
For the what?
Fixed :)
| ctx.snowflakeLock.Lock() | ||
| numSnowflakes := ctx.snowflakes.Len() | ||
| ctx.snowflakeLock.Unlock() | ||
| if numSnowflakes <= 0 { | ||
| ctx.metrics.clientDeniedCount++ |
Should this increment also be protected by a lock? Or is it impossible for two goroutines to execute it simultaneously?
Thanks for pointing this out. There is a chance for a race here if the counts are updated at the same time that the metrics are being logged (this happens once every 24 hours).
I added a different lock to the metrics struct to handle this in a new commit. How does it look?
There's a race condition in the broker where both the proxy and the client processes try to pop/remove the same snowflake from the heap. This patch adds synchronization to prevent simultaneous accesses to snowflakes.
We had some data races in the broker that occur when proxies and clients modify the heap/snowflake map at the same time. This test has a client and proxy access the broker simultaneously to check for data races.
Added another lock to the metrics struct to synchronize accesses to the broker stats. There's a possible race condition if stats are updated at the same time they are being logged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
There's a race condition in the broker where both the proxy and the
client processes try to pop/remove the same snowflake from the heap.
This patch adds synchronization to prevent simultaneous accesses to
snowflakes.
The text was updated successfully, but these errors were encountered: