Skip to content

Commit 07d2571

Browse files
committed
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
1 parent 8c51315 commit 07d2571

File tree

1 file changed

+39
-25
lines changed

1 file changed

+39
-25
lines changed

spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -603,38 +603,52 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
603603
resizing = true;
604604
}
605605

606-
// Either create a new table or reuse the existing one
607-
Reference<K, V>[] restructured =
608-
(resizing ? createReferenceArray(restructureSize) : this.references);
609-
610-
// Restructure
611606
int newCount = 0;
612-
for (int i = 0; i < this.references.length; i++) {
613-
ref = this.references[i];
614-
if (!resizing) {
615-
restructured[i] = null;
616-
}
617-
while (ref != null) {
618-
if (!toPurge.contains(ref)) {
619-
Entry<K, V> entry = ref.get();
620-
// Also filter out null references that are now null
621-
// they should be polled the queue in a later restructure call.
622-
if (entry != null) {
623-
int index = getIndex(ref.getHash(), restructured);
624-
restructured[index] = this.referenceManager.createReference(
625-
entry, ref.getHash(), restructured[index]);
626-
newCount++;
607+
// Restructure the resized reference array
608+
if (resizing) {
609+
Reference<K, V>[] restructured = createReferenceArray(restructureSize);
610+
for (int i = 0; i < this.references.length; i++) {
611+
ref = this.references[i];
612+
while (ref != null) {
613+
if (!toPurge.contains(ref)) {
614+
Entry<K, V> entry = ref.get();
615+
// Also filter out null references that are now null
616+
// they should be polled from the queue in a later restructure call.
617+
if (entry != null) {
618+
int index = getIndex(ref.getHash(), restructured);
619+
restructured[index] = this.referenceManager.createReference(
620+
entry, ref.getHash(), restructured[index]);
621+
newCount++;
622+
}
627623
}
624+
ref = ref.getNext();
628625
}
629-
ref = ref.getNext();
630626
}
631-
}
632-
633-
// Replace volatile members
634-
if (resizing) {
627+
// Replace volatile members
635628
this.references = restructured;
636629
this.resizeThreshold = (int) (this.references.length * getLoadFactor());
637630
}
631+
// Restructure the existing reference array "in place"
632+
else {
633+
for (int i = 0; i < this.references.length; i++) {
634+
Reference<K, V> purgedRef = null;
635+
ref = this.references[i];
636+
while (ref != null) {
637+
if (!toPurge.contains(ref)) {
638+
Entry<K, V> entry = ref.get();
639+
// Also filter out null references that are now null
640+
// they should be polled from the queue in a later restructure call.
641+
if (entry != null) {
642+
purgedRef = this.referenceManager.createReference(
643+
entry, ref.getHash(), purgedRef);
644+
}
645+
newCount++;
646+
}
647+
ref = ref.getNext();
648+
}
649+
this.references[i] = purgedRef;
650+
}
651+
}
638652
this.count.set(Math.max(newCount, 0));
639653
}
640654
finally {

0 commit comments

Comments
 (0)