0
1
mirror of https://github.com/golang/go synced 2025-09-11 06:29:43 +00:00

internal/concurrent: handle boundary case for hash bits in HashTrieMap

Currently the HashTrieMap has a panic for running out of hash bits, but
it turns out we can end up in these paths in valid cases, like inserting
or deleting an element that requires *all* the hash bits to finds its
position in the tree. There's basically an off-by-one error here where
the panic fires erroneously.

This wasn't caught before the original CL landed because it's very
unlikely on 64-bit platforms, with a 64-bit hash, but much more likely
on 32-bit platforms, where using all 32 bits of a 32-bit hash is much
more likely.

This CL makes the condition for panicking much more explicit, which
avoids the off-by-one error.

After this CL, I can't get the tests to fail on 32-bit under stress
testing.

Change-Id: I855e301e3b3893e2b6b017f6dd9f3d83a94a558d
Reviewed-on: https://go-review.googlesource.com/c/go/+/580138
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
This commit is contained in:
Michael Anthony Knyszek
2024-04-19 04:41:01 +00:00
committed by Gopher Robot
parent 2ff89341f6
commit 4324d5a88c

View File

@@ -78,6 +78,7 @@ func (ht *HashTrieMap[K, V]) LoadOrStore(key K, value V) (result V, loaded bool)
// Find the key or a candidate location for insertion.
i = ht.root
hashShift = 8 * goarch.PtrSize
haveInsertPoint := false
for hashShift != 0 {
hashShift -= nChildrenLog2
@@ -85,6 +86,7 @@ func (ht *HashTrieMap[K, V]) LoadOrStore(key K, value V) (result V, loaded bool)
n = slot.Load()
if n == nil {
// We found a nil slot which is a candidate for insertion.
haveInsertPoint = true
break
}
if n.isEntry {
@@ -94,11 +96,12 @@ func (ht *HashTrieMap[K, V]) LoadOrStore(key K, value V) (result V, loaded bool)
if v, ok := n.entry().lookup(key, ht.keyEqual); ok {
return v, true
}
haveInsertPoint = true
break
}
i = n.indirect()
}
if hashShift == 0 {
if !haveInsertPoint {
panic("internal/concurrent.HashMapTrie: ran out of hash bits while iterating")
}
@@ -188,6 +191,7 @@ func (ht *HashTrieMap[K, V]) CompareAndDelete(key K, old V) (deleted bool) {
// Find the key or return when there's nothing to delete.
i = ht.root
hashShift = 8 * goarch.PtrSize
found := false
for hashShift != 0 {
hashShift -= nChildrenLog2
@@ -204,11 +208,12 @@ func (ht *HashTrieMap[K, V]) CompareAndDelete(key K, old V) (deleted bool) {
return
}
// We've got something to delete.
found = true
break
}
i = n.indirect()
}
if hashShift == 0 {
if !found {
panic("internal/concurrent.HashMapTrie: ran out of hash bits while iterating")
}
@@ -248,7 +253,7 @@ func (ht *HashTrieMap[K, V]) CompareAndDelete(key K, old V) (deleted bool) {
// Check if the node is now empty (and isn't the root), and delete it if able.
for i.parent != nil && i.empty() {
if hashShift == 64 {
if hashShift == 8*goarch.PtrSize {
panic("internal/concurrent.HashMapTrie: ran out of hash bits while iterating")
}
hashShift += nChildrenLog2