HBASE-27648 CopyOnWriteArrayMap does not honor contract of ConcurrentMap.putIfAbsent (#5031)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
This commit is contained in:
Bryan Beaudreault 2023-02-17 19:17:52 -05:00 committed by Bryan Beaudreault
parent 9889ee2100
commit e845e2e4dc
2 changed files with 16 additions and 2 deletions

View File

@ -332,7 +332,8 @@ public class CopyOnWriteArrayMap<K, V> extends AbstractMap<K, V>
if (index < 0) {
COWEntry<K, V> newEntry = new COWEntry<>(key, value);
this.holder = current.insert(-(index + 1), newEntry);
return value;
// putIfAbsent contract requires returning null if no previous entry exists
return null;
}
return current.entries[index].getValue();
}

View File

@ -41,6 +41,7 @@ public class TestCopyOnWriteMaps {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestCopyOnWriteMaps.class);
private static final int MAX_INITIAL_ENTRIES = 10_000;
private static final int MAX_RAND = 10 * 1000 * 1000;
private ConcurrentNavigableMap<Long, Long> m;
private ConcurrentSkipListMap<Long, Long> csm;
@ -50,7 +51,7 @@ public class TestCopyOnWriteMaps {
m = new CopyOnWriteArrayMap<>();
csm = new ConcurrentSkipListMap<>();
for (long i = 0; i < 10000; i++) {
for (long i = 0; i < MAX_INITIAL_ENTRIES; i++) {
long o = ThreadLocalRandom.current().nextLong(MAX_RAND);
m.put(i, o);
csm.put(i, o);
@ -60,6 +61,18 @@ public class TestCopyOnWriteMaps {
csm.put(0L, o);
}
@Test
public void testPutIfAbsent() {
long key = MAX_INITIAL_ENTRIES * 2;
long val = ThreadLocalRandom.current().nextLong(MAX_RAND);
// initial call should return null, then should return previous value
assertNull(m.putIfAbsent(key, val));
assertEquals(val, (long) m.putIfAbsent(key, val * 2));
// test same with csm so we ensure similar semantics
assertNull(csm.putIfAbsent(key, val));
assertEquals(val, (long) csm.putIfAbsent(key, val * 2));
}
@Test
public void testSize() {
assertEquals("Size should always be equal", m.size(), csm.size());