Fix data race on chunks shared by DedupQueue#372
Merged
Conversation
DedupQueue hands the same *Chunk pointer to every caller waiting on a deduplicated GetChunk request. Chunk lazily materializes its plain data and ID on first access without any locking, so concurrent callers that trigger that conversion race on the chunk's internal fields. This is reachable on a read-only chunk-server whenever the served format differs from the store format, e.g. an uncompressed server backed by a compressed store. Give every caller, including the one that performed the upstream request, a shallow copy of the chunk instead. The copies share the underlying read-only data and storage slices, so no chunk data is duplicated; only the lazily-calculated fields become private to each caller. The chunk held by the in-flight request is never handed out directly and stays untouched while waiters read from it. The new test fails with -race (and on the shared-pointer assertion) without this fix.
folbricht
added a commit
that referenced
this pull request
Jul 10, 2026
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.
DedupQueue returns the same
*Chunkpointer to every caller waiting on a coalescedGetChunkrequest.Chunk.Data()andChunk.ID()are lazy memoizers that write the chunk's internal fields without synchronization, so two concurrent callers that trigger the storage-to-plain conversion race onc.data/c.id. Reachable today on a read-only chunk-server whose served format differs from the upstream store format (e.g.chunk-server -uover a compressed store), and it becomes the standard configuration once chunk encryption (#371) lands, since an encrypting server always has a converter mismatch with its store.Fix: hand every caller a shallow copy of the chunk. The copies share the underlying read-only
data/storageslices — no chunk data is duplicated — while the lazily-calculated fields become private to each caller. The master chunk held by the in-flight request is never handed out directly, so it stays untouched while waiters read it; the leader publishes it viamarkDone(channel close) before anyone copies it, which provides the necessary happens-before edge.The added test requests the same chunk from 10 goroutines and materializes the plain data in each: without the fix it fails under
-race(write/write race onc.data) and on the shared-pointer assertion; with the fix the full library suite passes under-race.