Repository navigation
Document that copy-on-write bitmaps need synchronization even for Clone - #595
Merged
Merged
Conversation
With copy-on-write enabled, rb.Clone() marks the containers of rb as shared, and x.Or(rb) / x.Xor(rb) do the same when x also uses copy-on-write. These calls look read-only but write internal flags of rb, so concurrent use from several goroutines is a data race. State this in the SetCopyOnWrite and Clone doc comments (32-bit and 64-bit) and in the README goroutine-safety section. Closes #528
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
With copy-on-write enabled, some operations that leave a bitmap's values unchanged still write its internal sharing flags (
needCopyOnWrite):rb.Clone()marks every container ofrbas shared.x.Or(rb)andx.Xor(rb)mark the containers ofrbthey share withx, whenxalso uses copy-on-write.So two goroutines that only call
rb.Clone()on the same copy-on-write bitmap race (#528). Static operations such asOr(x1, x2),FlipandFastOronly read these flags, so they still race with a concurrentClone().The existing doc comment already said that copy-on-write "requires extra care in a threaded context", and the README already says bitmaps are not synchronized. This PR states the specific case plainly instead of adding locking to the hot paths. Adding a lock to
Clonealone (as in #574) would leave the other paths racing. Locking every flag write would slow down the binary operations for all copy-on-write users.Changes Made
SetCopyOnWriteandClonedoc comments inroaring.goandroaring64/roaring64.gonow explain which operations write the flags, and that concurrent use (even onlyClone) needs external synchronization.No code changes.
Testing
Documentation only.
go test ./...passes. I checked withgo test -racethat concurrentClone/CloneandClone/Oron a copy-on-write bitmap race on master. I checked thatClone,x.Or(rb)andx.Xor(rb)setrb's flags in both the 32-bit and 64-bit packages.Related Issues
Closes #528