Skip to content

Commit fe1c2f9

Browse files
Fix race condition
1 parent 768e037 commit fe1c2f9

File tree

2 files changed

+32
-4
lines changed

2 files changed

+32
-4
lines changed

x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/KeyMapping.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.elasticsearch.blobcache.shared;
99

1010
import java.util.concurrent.ConcurrentHashMap;
11+
import java.util.concurrent.atomic.AtomicReference;
1112
import java.util.function.BiConsumer;
1213
import java.util.function.Function;
1314

@@ -37,12 +38,13 @@ public Value get(Key1 key1, Key2 key2) {
3738
* @return the resulting value.
3839
*/
3940
public Value computeIfAbsent(Key1 key1, Key2 key2, Function<Key2, Value> function) {
40-
var inner = mapping.compute(key1, (k, current) -> {
41+
AtomicReference<Value> result = new AtomicReference<>();
42+
mapping.compute(key1, (k, current) -> {
4143
ConcurrentHashMap<Key2, Value> map = current == null ? new ConcurrentHashMap<>() : current;
42-
map.computeIfAbsent(key2, function);
44+
result.setPlain(map.computeIfAbsent(key2, function));
4345
return map;
4446
});
45-
return inner.get(key2);
47+
return result.getPlain();
4648
}
4749

4850
public boolean remove(Key1 key1, Key2 key2, Value value) {

x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/KeyMappingTests.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void testMultiThreaded() {
6262
for (int j = 0; j < 1000; ++j) {
6363
Integer finalJ = j;
6464
assertNull(mapping.get(k1, k2));
65-
mapping.computeIfAbsent(k1, k2, (kx) -> finalJ);
65+
assertSame(finalJ, mapping.computeIfAbsent(k1, k2, (kx) -> finalJ));
6666
assertEquals(finalJ, mapping.get(k1, k2));
6767
assertTrue(mapping.remove(k1, k2, finalJ));
6868
if ((j & 1) == 0) {
@@ -84,4 +84,30 @@ public void testMultiThreaded() {
8484

8585
assertEquals(Set.of(), mapping.key1s());
8686
}
87+
88+
public void testMultiThreadedSameKey() {
89+
final String k1 = randomAlphanumericOfLength(10);
90+
KeyMapping<String, String, Integer> mapping = new KeyMapping<>();
91+
92+
List<Thread> threads = IntStream.range(0, 10).mapToObj(i -> new Thread(() -> {
93+
for (int j = 0; j < 1000; ++j) {
94+
Integer computeValue = i * 1000 + j;
95+
Integer value = mapping.computeIfAbsent(k1, k1, (kx) -> computeValue);
96+
assertNotNull(value);
97+
// either our value or another threads value.
98+
assertTrue(value == computeValue || value / 1000 != i);
99+
mapping.remove(k1, k1, value);
100+
}
101+
})).toList();
102+
threads.forEach(Thread::start);
103+
threads.forEach(t -> {
104+
try {
105+
t.join(10000);
106+
} catch (InterruptedException e) {
107+
throw new RuntimeException(e);
108+
}
109+
});
110+
111+
assertEquals(Set.of(), mapping.key1s());
112+
}
87113
}

0 commit comments

Comments
 (0)