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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a race in
destroyHandler
w.r.t.dc.messages
sync.MapMotivation:
This PR is meant to fix a contributing factor to the following race PR: jjeffcaii/reactor-go#45
In the current implementation of
destroyHandler
- it's possible to callstopWithError
on a callback that has already been unregistered. This can affect a completely unrelatedDuplex
object (because global pools are used forprocessor
objects - and an already "disposed" processor can receive an error).The race (for example) can occur between the following pieces of code:
rsocket-go/internal/socket/duplex.go
Lines 163 to 173 in 099cb5b
rsocket-go/internal/socket/duplex.go
Lines 257 to 266 in 099cb5b
They both access
dc.messages
.destroyHandler
- for reading (iteration),onFinally
- for writing (insideunregister
method).In the existing version of
destroyHandler
- by the time iteration (Range()
) overdc.messages
is complete - some of the messages may have already been unregistered and removed from the map. This is where the race comes from.As per documentation: https://pkg.go.dev/sync#Map.Range
-> Meaning that it's totally safe to invoke every callback from inside
Range
- no need to store callbacks in a slice beforehand.Modifications:
Perform callback
stopWithError
invocations directly duringRange
iteration, without creating a slice of callbacks beforehand.Result:
The issue described in jjeffcaii/reactor-go#45 goes away.