collate: make Collator.Compare and CompareString goroutine-safe#60
collate: make Collator.Compare and CompareString goroutine-safe#60carli2 wants to merge 2 commits intogolang:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Replace golang.org/x/text with our fork carli2/text@fix-collator-data-race. The upstream Collator.CompareString/_Compare mutates shared _iter[0]/_iter[1] fields, causing a data race when a single Collator is used as a Less() closure across parallel shard scans. Fix: iter variables are now goroutine-local (stack-allocated) in Compare, CompareString, getColElems and getColElemsString. Collator is immutable after construction. Upstream PR: golang/text#60 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c71dedc to
25ccbef
Compare
|
This PR (HEAD: 25ccbef) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/text/+/751320. Important tips:
|
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/751320. |
Collator stored two iterator slots as struct fields (_iter [2]iter). Both Compare and CompareString wrote into these shared fields, making concurrent calls on the same Collator instance unsafe. Remove _iter [2]iter from Collator. Compare, CompareString, getColElems, and getColElemsString now declare their iter as a goroutine-local variable. Each goroutine gets its own stack-allocated scratch buffer (wa [512]colltab.Elem); the CPU only loads the cache lines the comparison actually touches, so early-exit comparisons pay no extra cost. Collator is now immutable after construction. Fixes golang/go#77927
25ccbef to
d42ea42
Compare
|
Message from Gopher Robot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/751320. |
|
This PR (HEAD: d42ea42) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/text/+/751320. Important tips:
|
|
This PR (HEAD: 20c7a40) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/text/+/751320. Important tips:
|
Collator stored two iterator slots as struct fields (_iter [2]iter).
Both Compare and CompareString wrote into these shared fields, making
concurrent calls on the same Collator instance unsafe.
Remove _iter [2]iter from Collator. Compare, CompareString,
getColElems, and getColElemsString now declare their iter as a
goroutine-local variable. Each goroutine gets its own stack-allocated
scratch buffer (wa [512]colltab.Elem); the CPU only loads the cache
lines the comparison actually touches, so early-exit comparisons pay
no extra cost. Collator is now immutable after construction.
Fixes golang/go#77927