Fix data race in BucketedBytes pool
#4792
Merged
Commits on Oct 20, 2021
-
Fix data race in BucketedBytes pool
Previous test didn't detect the data race: we copied the bytes header to the bytes.Buffer so when appending to the slice we were not modifying the original one. However, the usage of this in bucketChunkReader.save() actually modifies the referenced slice, so the test was modified to test that it can be done safely. The race condition happened because we were reading the referenced slice capacity after putting it back to the pool, when someone else might already retrieved and modified it. Before modifying the implementation, this was the data race reported: ================== WARNING: DATA RACE Read at 0x00c0000bc900 by goroutine 36: github.com/thanos-io/thanos/pkg/pool.(*BucketedBytes).Put() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool.go:124 +0x1f9 github.com/thanos-io/thanos/pkg/pool.TestRacePutGet.func1() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:108 +0xfa github.com/thanos-io/thanos/pkg/pool.TestRacePutGet·dwrap·3() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:119 +0x65 Previous write at 0x00c0000bc900 by goroutine 27: github.com/thanos-io/thanos/pkg/pool.TestRacePutGet.func1() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:94 +0x1fa github.com/thanos-io/thanos/pkg/pool.TestRacePutGet·dwrap·3() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:119 +0x65 Goroutine 36 (running) created at: github.com/thanos-io/thanos/pkg/pool.TestRacePutGet() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:119 +0x257 testing.tRunner() /usr/local/Cellar/go/1.17/libexec/src/testing/testing.go:1259 +0x22f testing.(*T).Run·dwrap·21() 1 Fix data race in BucketedBytes pool /usr/local/Cellar/go/1.17/libexec/src/testing/testing.go:1306 +0x47 Goroutine 27 (running) created at: github.com/thanos-io/thanos/pkg/pool.TestRacePutGet() /Users/oleg/w/github.com/thanos-io/thanos/pkg/pool/pool_test.go:119 +0x257 testing.tRunner() /usr/local/Cellar/go/1.17/libexec/src/testing/testing.go:1259 +0x22f testing.(*T).Run·dwrap·21() /usr/local/Cellar/go/1.17/libexec/src/testing/testing.go:1306 +0x47 ================== Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> -
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
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.