Fix data race on chunks shared by WriteDedupQueue#373
Merged
Conversation
WriteDedupQueue.GetChunk hands the chunk of an in-flight StoreChunk request for the same ID to every waiting reader. The chunk published in the request is the one the storing caller passed in and keeps using, and Chunk lazily materializes its plain data without locking, so concurrent readers race with each other and with the writer on the chunk's internal fields. Apply the same fix as for DedupQueue.GetChunk in #372: StoreChunk publishes a shallow copy detached from the caller's chunk, and every waiting reader receives its own copy of that. The copies share the underlying read-only data, so no chunk data is duplicated. The new test fails with -race (and on the shared-pointer assertion) without this fix.
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
1 participant
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.
Follow-up to #372, which fixed the same defect in
DedupQueue.GetChunkbut missed the sibling path:WriteDedupQueue.GetChunkreturns the chunk of an in-flightStoreChunkrequest for the same ID to every waiting reader. That chunk is the exact*Chunkthe storing caller passed in and continues to use, andChunk.Data()/ID()memoize internal fields without synchronization — so concurrent readers race with each other and with the writer.Same fix shape as #372:
StoreChunkpublishes a shallow copy detached from the caller's chunk viamarkDone, and each waiting reader receives its own copy of that master. The master is never handed out or mutated,markDone's channel close provides the happens-before edge, and the copies share the underlying read-only data so nothing is duplicated.The added test stores a chunk while ten readers request the same ID (deterministically interleaved with
testing/synctest): without the fix it fails under-raceand on the shared-pointer assertion; with the fix the full library suite passes under-race.