Skip to content

[Fix] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException #4771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merlimat merged 7 commits into from
May 3, 2026
Merged

[Fix] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException #4771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0ca0507
fix ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException
void-ptr974 May 1, 2026
21f33fb
fix ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException
void-ptr974 May 1, 2026
1f2d311
fix ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException
void-ptr974 May 1, 2026
88a055f
Merge branch 'apache:master' into fix_clhm_aoob
void-ptr974 May 1, 2026
97af802
fix ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException
void-ptr974 May 1, 2026
ab0bdb8
Merge remote-tracking branch 'refs/remotes/void-ptr/fix_clhm_aoob' in…
void-ptr974 May 1, 2026
bdc191e
fix ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException
void-ptr974 May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 62 additions & 47 deletions ...er-server/src/main/java/org/apache/bookkeeper/util/collections/ConcurrentLongHashMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.Lists;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.locks.StampedLock;
Expand Down Expand Up @@ -197,7 +198,7 @@ long getUsedBucketCount() {
public long capacity() {
long capacity = 0;
for (Section<V> s : sections) {
capacity += s.capacity;
capacity += s.table.capacity();
}
return capacity;
}
Expand Down Expand Up @@ -310,13 +311,18 @@ public interface EntryProcessor<V> {
void accept(long key, V value);
}

// A section is a portion of the hash map that is covered by a single
// A section is a portion of the hash map that is covered by a single lock. The keys, values
// and capacity arrays are bundled into an immutable Table snapshot so that readers always see
// a consistent (key, value, length) triple, eliminating the partial-publish race that the
// previous design had to paper over with Math.min(keys.length, values.length).
@SuppressWarnings("serial")
private static final class Section<V> extends StampedLock {
private volatile long[] keys;
private volatile V[] values;
private record Table<V>(long[] keys, V[] values, int capacity) { }

// Section is Serializable only by inheritance from StampedLock; never actually serialized.
@SuppressFBWarnings("SE_BAD_FIELD")
private volatile Table<V> table;

private volatile int capacity;
private final int initCapacity;
private volatile int size;
private int usedBuckets;
Expand All @@ -330,31 +336,29 @@ private static final class Section<V> extends StampedLock {

Section(int capacity, float mapFillFactor, float mapIdleFactor, boolean autoShrink,
float expandFactor, float shrinkFactor) {
this.capacity = alignToPowerOfTwo(capacity);
this.initCapacity = this.capacity;
this.keys = new long[this.capacity];
this.values = (V[]) new Object[this.capacity];
int initial = alignToPowerOfTwo(capacity);
this.initCapacity = initial;
this.table = new Table<>(new long[initial], (V[]) new Object[initial], initial);
this.size = 0;
this.usedBuckets = 0;
this.autoShrink = autoShrink;
this.mapFillFactor = mapFillFactor;
this.mapIdleFactor = mapIdleFactor;
this.expandFactor = expandFactor;
this.shrinkFactor = shrinkFactor;
this.resizeThresholdUp = (int) (this.capacity * mapFillFactor);
this.resizeThresholdBelow = (int) (this.capacity * mapIdleFactor);
this.resizeThresholdUp = (int) (initial * mapFillFactor);
this.resizeThresholdBelow = (int) (initial * mapIdleFactor);
}

V get(long key, int keyHash) {

long stamp = tryOptimisticRead();
boolean acquiredLock = false;

// add local variable here, so OutOfBound won't happen
long[] keys = this.keys;
V[] values = this.values;
// calculate table.length as capacity to avoid rehash changing capacity
int bucket = signSafeMod(keyHash, values.length);
Table<V> table = this.table;
long[] keys = table.keys();
V[] values = table.values();
int bucket = signSafeMod(keyHash, table.capacity());

try {
while (true) {
Expand All @@ -377,10 +381,10 @@ V get(long key, int keyHash) {
stamp = readLock();
acquiredLock = true;

// update local variable
keys = this.keys;
values = this.values;
bucket = signSafeMod(keyHash, values.length);
table = this.table;
keys = table.keys();
values = table.values();
bucket = signSafeMod(keyHash, table.capacity());
storedKey = keys[bucket];
storedValue = values[bucket];
}
Expand All @@ -393,7 +397,7 @@ V get(long key, int keyHash) {
}
}

bucket = (bucket + 1) & (values.length - 1);
bucket = (bucket + 1) & (table.capacity() - 1);
}
} finally {
if (acquiredLock) {
Expand All @@ -406,7 +410,10 @@ V put(long key, V value, int keyHash, boolean onlyIfAbsent, LongFunction<V> valu
int bucket = keyHash;

long stamp = writeLock();
int capacity = this.capacity;
Table<V> table = this.table;
long[] keys = table.keys();
V[] values = table.values();
int capacity = table.capacity();

// Remember where we find the first available spot
int firstDeletedKey = -1;
Expand Down Expand Up @@ -474,6 +481,9 @@ V put(long key, V value, int keyHash, boolean onlyIfAbsent, LongFunction<V> valu
private void cleanDeletedStatus(int startBucket) {
// Cleanup all the buckets that were in `DeletedValue` state,
// so that we can reduce unnecessary expansions
Table<V> table = this.table;
V[] values = table.values();
int capacity = table.capacity();
int lastBucket = signSafeMod(startBucket - 1, capacity);
while (values[lastBucket] == DeletedValue) {
values[lastBucket] = (V) EmptyValue;
Expand All @@ -486,10 +496,13 @@ private void cleanDeletedStatus(int startBucket) {
private V remove(long key, Object value, int keyHash) {
int bucket = keyHash;
long stamp = writeLock();
Table<V> table = this.table;
long[] keys = table.keys();
V[] values = table.values();
int capacity = table.capacity();

try {
while (true) {
int capacity = this.capacity;
bucket = signSafeMod(bucket, capacity);

long storedKey = keys[bucket];
Expand Down Expand Up @@ -551,7 +564,10 @@ int removeIf(LongObjectPredicate<V> filter) {
int removedCount = 0;
try {
// Go through all the buckets for this section
int capacity = this.capacity;
Table<V> table = this.table;
long[] keys = table.keys();
V[] values = table.values();
int capacity = table.capacity();
for (int bucket = 0; size > 0 && bucket < capacity; bucket++) {
long storedKey = keys[bucket];
V storedValue = values[bucket];
Expand Down Expand Up @@ -583,6 +599,7 @@ int removeIf(LongObjectPredicate<V> filter) {
// so as to avoid frequent shrinking and expansion near initCapacity,
// frequent shrinking and expansion,
// additionally opened arrays will consume more memory and affect GC
int capacity = this.table.capacity();
int newCapacity = Math.max(alignToPowerOfTwo((int) (capacity / shrinkFactor)), initCapacity);
int newResizeThresholdUp = (int) (newCapacity * mapFillFactor);
if (newCapacity < capacity && newResizeThresholdUp > size) {
Expand All @@ -602,11 +619,12 @@ void clear() {
long stamp = writeLock();

try {
if (autoShrink && capacity > initCapacity) {
Table<V> table = this.table;
if (autoShrink && table.capacity() > initCapacity) {
shrinkToInitCapacity();
} else {
Arrays.fill(keys, 0);
Arrays.fill(values, EmptyValue);
Arrays.fill(table.keys(), 0);
Arrays.fill(table.values(), EmptyValue);
this.size = 0;
this.usedBuckets = 0;
}
Expand All @@ -618,9 +636,10 @@ void clear() {
public void forEach(EntryProcessor<V> processor) {
long stamp = tryOptimisticRead();

int capacity = this.capacity;
long[] keys = this.keys;
V[] values = this.values;
Table<V> table = this.table;
int capacity = table.capacity();
long[] keys = table.keys();
V[] values = table.values();

boolean acquiredReadLock = false;

Expand All @@ -632,9 +651,10 @@ public void forEach(EntryProcessor<V> processor) {
stamp = readLock();
acquiredReadLock = true;

capacity = this.capacity;
keys = this.keys;
values = this.values;
table = this.table;
capacity = table.capacity();
keys = table.keys();
values = table.values();
}

// Go through all the buckets for this section
Expand Down Expand Up @@ -666,6 +686,9 @@ private void rehash(int newCapacity) {
// Expand the hashmap
long[] newKeys = new long[newCapacity];
V[] newValues = (V[]) new Object[newCapacity];
Table<V> table = this.table;
long[] keys = table.keys();
V[] values = table.values();

// Re-hash table
for (int i = 0; i < keys.length; i++) {
Expand All @@ -676,29 +699,21 @@ private void rehash(int newCapacity) {
}
}

keys = newKeys;
values = newValues;
this.table = new Table<>(newKeys, newValues, newCapacity);
usedBuckets = size;
// Capacity needs to be updated after the values, so that we won't see
// a capacity value bigger than the actual array size
capacity = newCapacity;
resizeThresholdUp = (int) (capacity * mapFillFactor);
resizeThresholdBelow = (int) (capacity * mapIdleFactor);
resizeThresholdUp = (int) (newCapacity * mapFillFactor);
resizeThresholdBelow = (int) (newCapacity * mapIdleFactor);
}

private void shrinkToInitCapacity() {
long[] newKeys = new long[initCapacity];
V[] newValues = (V[]) new Object[initCapacity];

keys = newKeys;
values = newValues;
table = new Table<>(newKeys, newValues, initCapacity);
size = 0;
usedBuckets = 0;
// Capacity needs to be updated after the values, so that we won't see
// a capacity value bigger than the actual array size
capacity = initCapacity;
resizeThresholdUp = (int) (capacity * mapFillFactor);
resizeThresholdBelow = (int) (capacity * mapIdleFactor);
resizeThresholdUp = (int) (initCapacity * mapFillFactor);
resizeThresholdBelow = (int) (initCapacity * mapIdleFactor);
}

private static <V> void insertKeyValueNoLock(long[] keys, V[] values, long key, V value) {
Expand Down
Loading
Loading