Avoid race conditions while restructuring ConcurrentReferenceHashMap

Prior to this commit, the `ConcurrentReferenceHashMap#restructure`
operation would null out the entire references array before starting the
restructuring operation (in case resizing is not necessary).

This could cause at runtime race conditions where a lookup operation
would return null, when the value is actually cached but not accesible
during the restructuring phase.

This commit ensures that, when resizing is not required, a new reference
list is built (purged of null entries) and then assigned to the
reference array. This way, concurrent reads will not return null for
existing entries and there are less chances of re-calculating cache
entries during the restructuring phase.

Closes gh-31008
This commit is contained in:
Brian Clozel 2023-12-11 11:57:58 +01:00
parent 8c51315cd6
commit 07d2571e0b
1 changed files with 41 additions and 27 deletions

View File

@ -603,38 +603,52 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
resizing = true;
}
// Either create a new table or reuse the existing one
Reference<K, V>[] restructured =
(resizing ? createReferenceArray(restructureSize) : this.references);
// Restructure
int newCount = 0;
for (int i = 0; i < this.references.length; i++) {
ref = this.references[i];
if (!resizing) {
restructured[i] = null;
}
while (ref != null) {
if (!toPurge.contains(ref)) {
Entry<K, V> entry = ref.get();
// Also filter out null references that are now null
// they should be polled the queue in a later restructure call.
if (entry != null) {
int index = getIndex(ref.getHash(), restructured);
restructured[index] = this.referenceManager.createReference(
entry, ref.getHash(), restructured[index]);
newCount++;
}
}
ref = ref.getNext();
}
}
// Replace volatile members
// Restructure the resized reference array
if (resizing) {
Reference<K, V>[] restructured = createReferenceArray(restructureSize);
for (int i = 0; i < this.references.length; i++) {
ref = this.references[i];
while (ref != null) {
if (!toPurge.contains(ref)) {
Entry<K, V> entry = ref.get();
// Also filter out null references that are now null
// they should be polled from the queue in a later restructure call.
if (entry != null) {
int index = getIndex(ref.getHash(), restructured);
restructured[index] = this.referenceManager.createReference(
entry, ref.getHash(), restructured[index]);
newCount++;
}
}
ref = ref.getNext();
}
}
// Replace volatile members
this.references = restructured;
this.resizeThreshold = (int) (this.references.length * getLoadFactor());
}
// Restructure the existing reference array "in place"
else {
for (int i = 0; i < this.references.length; i++) {
Reference<K, V> purgedRef = null;
ref = this.references[i];
while (ref != null) {
if (!toPurge.contains(ref)) {
Entry<K, V> entry = ref.get();
// Also filter out null references that are now null
// they should be polled from the queue in a later restructure call.
if (entry != null) {
purgedRef = this.referenceManager.createReference(
entry, ref.getHash(), purgedRef);
}
newCount++;
}
ref = ref.getNext();
}
this.references[i] = purgedRef;
}
}
this.count.set(Math.max(newCount, 0));
}
finally {