Skip to content

Commit cb64820

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 dbbacd6 commit cb64820

File tree

2 files changed

+590
-11
lines changed

2 files changed

+590
-11
lines changed

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -237,26 +237,34 @@ public void close() {
237237
* closed
238238
* @since 6.0
239239
*/
240+
@SuppressWarnings("ReferenceEquality")
240241
@API(status = MAINTAINED, since = "6.0")
241242
public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? extends V> defaultCreator) {
242-
Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
243-
CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
244-
StoredValue storedValue = getStoredValue(compositeKey);
243+
requireNonNull(defaultCreator);
244+
var compositeKey = new CompositeKey<>(namespace, key);
245+
var storedValue = getStoredValue(compositeKey);
245246
var result = StoredValue.evaluateIfNotNull(storedValue);
246247
if (result == null) {
247-
StoredValue newStoredValue = this.storedValues.compute(compositeKey, (__, oldStoredValue) -> {
248-
if (StoredValue.evaluateIfNotNull(oldStoredValue) == null) {
248+
var newStoredValue = this.storedValues.compute(compositeKey, (__, oldStoredValue) -> {
249+
if (oldStoredValue == storedValue || oldStoredValue == null) {
249250
rejectIfClosed();
250-
var computedValue = Preconditions.notNull(defaultCreator.apply(key),
251-
"defaultCreator must not return null");
252-
return newStoredValue(() -> {
251+
return newStoredValue(new MemoizingSupplier(() -> {
253252
rejectIfClosed();
254-
return computedValue;
255-
});
253+
return requireNonNull(defaultCreator.apply(key));
254+
}));
256255
}
257256
return oldStoredValue;
258257
});
259-
return requireNonNull(newStoredValue.evaluate());
258+
try {
259+
return requireNonNull(newStoredValue.evaluate());
260+
}
261+
catch (Throwable t) {
262+
// If the evaluation fails, remove the failed entry so subsequent calls can retry
263+
if (newStoredValue == this.storedValues.get(compositeKey)) {
264+
this.storedValues.remove(compositeKey, newStoredValue);
265+
}
266+
throw t;
267+
}
260268
}
261269
return result;
262270
}

0 commit comments

Comments
 (0)