Skip to content

Commit d87f48c

Browse files
author
Vincent Potucek
committed
[tryout fix] concurrency bug in NamespacedHierarchicalStore#computeIfAbsent(Object, Object, Function) #5171 #5209
Signed-off-by: Vincent Potucek <[email protected]>
1 parent c9454e3 commit d87f48c

File tree

3 files changed

+711
-14
lines changed

3 files changed

+711
-14
lines changed

documentation/modules/ROOT/partials/release-notes/release-notes-6.0.2.adoc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ repository on GitHub.
1717
==== Bug Fixes
1818

1919
* Make `ConsoleLauncher` compatible with JDK 26 by avoiding final field mutations.
20+
* Fix a concurrency bug in `NamespacedHierarchicalStore#computeIfAbsent(Object, Object, Function)` where
21+
the `defaultCreator` function was executed while holding the store's internal
22+
map lock. Under parallel execution, this could cause threads using the store to
23+
block each other and temporarily see a missing or incorrectly initialized state
24+
for values created via `computeIfAbsent`. The method now evaluates
25+
`defaultCreator` outside the critical section using a memorizing supplier,
26+
aligning its behavior with the deprecated `getOrComputeIfAbsent`.
2027

2128
[[v6.0.2-junit-platform-deprecations-and-breaking-changes]]
2229
==== Deprecations and Breaking Changes

junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -240,27 +240,35 @@ public void close() {
240240
@API(status = MAINTAINED, since = "6.0")
241241
public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? extends V> defaultCreator) {
242242
Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
243-
CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
244-
StoredValue storedValue = getStoredValue(compositeKey);
243+
var compositeKey = new CompositeKey<>(namespace, key);
244+
var storedValue = getStoredValue(compositeKey);
245245
var result = StoredValue.evaluateIfNotNull(storedValue);
246246
if (result == null) {
247-
StoredValue newStoredValue = this.storedValues.compute(compositeKey, (__, oldStoredValue) -> {
248-
if (StoredValue.evaluateIfNotNull(oldStoredValue) == null) {
249-
rejectIfClosed();
250-
var computedValue = Preconditions.notNull(defaultCreator.apply(key),
251-
"defaultCreator must not return null");
252-
return newStoredValue(() -> {
253-
rejectIfClosed();
254-
return computedValue;
255-
});
247+
var value = storedValues.compute(compositeKey,
248+
(__, currentValue) -> currentValue == null || currentValue.equals(storedValue)
249+
? storeNewValue(key, defaultCreator)
250+
: currentValue);
251+
try {
252+
return requireNonNull(value.evaluate());
253+
}
254+
catch (Throwable t) { // remove failed entry to allow retry.
255+
if (value.equals(storedValues.get(compositeKey))) {
256+
storedValues.remove(compositeKey, value);
256257
}
257-
return oldStoredValue;
258-
});
259-
return requireNonNull(newStoredValue.evaluate());
258+
throw t;
259+
}
260260
}
261261
return result;
262262
}
263263

264+
private <K, V> StoredValue storeNewValue(K key, Function<? super K, ? extends V> defaultCreator) {
265+
rejectIfClosed();
266+
return newStoredValue(new MemoizingSupplier(() -> {
267+
rejectIfClosed();
268+
return requireNonNull(defaultCreator.apply(key));
269+
}));
270+
}
271+
264272
/**
265273
* Get the value stored for the supplied namespace and key in this store or
266274
* the parent store, if present, or call the supplied function to compute it

0 commit comments

Comments
 (0)