Skip to content

Commit 32faf28

Browse files
Approach 2 using DeferredSupplier with FutureTask
Signed-off-by: martinfrancois <f.martin@fastmail.com>
1 parent 267e938 commit 32faf28

File tree

1 file changed

+128
-42
lines changed

1 file changed

+128
-42
lines changed

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

Lines changed: 128 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.util.Optional;
2626
import java.util.concurrent.ConcurrentHashMap;
2727
import java.util.concurrent.ConcurrentMap;
28+
import java.util.concurrent.ExecutionException;
29+
import java.util.concurrent.FutureTask;
2830
import java.util.concurrent.atomic.AtomicInteger;
2931
import java.util.function.Function;
3032
import java.util.function.Supplier;
@@ -237,32 +239,82 @@ public void close() {
237239
* closed
238240
* @since 6.0
239241
*/
240-
@SuppressWarnings("ReferenceEquality")
241242
@API(status = MAINTAINED, since = "6.0")
242243
public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? extends V> defaultCreator) {
243244
Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
244245
CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
245-
StoredValue storedValue = getStoredValue(compositeKey);
246-
var result = StoredValue.evaluateIfNotNull(storedValue);
247-
if (result == null) {
248-
StoredValue newStoredValue = storedValues.compute(compositeKey, (__, oldStoredValue) -> {
249-
if (oldStoredValue == null || oldStoredValue == storedValue) {
250-
return newStoredValue(new MemoizingSupplier(() -> {
251-
rejectIfClosed();
252-
return Preconditions.notNull(defaultCreator.apply(key), "defaultCreator must not return null");
253-
}, true));
246+
247+
// CAS-retry loop: retry if another thread concurrently modifies the entry
248+
for (;;) {
249+
StoredValue localStoredValue = this.storedValues.get(compositeKey);
250+
if (localStoredValue != null) {
251+
Object localValue = evaluateForComputeIfAbsent(compositeKey, localStoredValue);
252+
if (localValue != null) {
253+
return localValue;
254254
}
255-
return oldStoredValue;
256-
});
255+
256+
Object computed = computeAndInstall(compositeKey, localStoredValue, key, defaultCreator);
257+
if (computed != null) {
258+
return computed;
259+
}
260+
continue;
261+
}
262+
263+
// No local mapping: consult parent first.
264+
if (this.parentStore != null) {
265+
StoredValue parentStoredValue = this.parentStore.getStoredValue(compositeKey);
266+
Object parentValue = StoredValue.evaluateIfNotNull(parentStoredValue);
267+
if (parentValue != null) {
268+
return parentValue;
269+
}
270+
}
271+
272+
Object computed = computeAndInstall(compositeKey, null, key, defaultCreator);
273+
if (computed != null) {
274+
return computed;
275+
}
276+
}
277+
}
278+
279+
private @Nullable Object evaluateForComputeIfAbsent(CompositeKey<N> compositeKey, StoredValue storedValue) {
280+
Supplier<@Nullable Object> supplier = storedValue.supplier();
281+
if (supplier instanceof DeferredSupplier deferred) {
282+
deferred.run();
257283
try {
258-
return requireNonNull(newStoredValue.evaluate());
284+
return deferred.getOrThrow();
259285
}
260286
catch (Throwable t) {
261-
storedValues.remove(compositeKey, newStoredValue);
287+
this.storedValues.remove(compositeKey, storedValue);
262288
throw t;
263289
}
264290
}
265-
return result;
291+
return storedValue.evaluate();
292+
}
293+
294+
private <K, V> @Nullable Object computeAndInstall(CompositeKey<N> compositeKey, @Nullable StoredValue expectedOld,
295+
K key, Function<? super K, ? extends V> defaultCreator) {
296+
297+
var deferred = new DeferredSupplier(() -> {
298+
rejectIfClosed();
299+
return Preconditions.notNull(defaultCreator.apply(key), "defaultCreator must not return null");
300+
});
301+
StoredValue newStoredValue = newStoredValue(deferred);
302+
303+
boolean installed = (expectedOld == null ? this.storedValues.putIfAbsent(compositeKey, newStoredValue) == null
304+
: this.storedValues.replace(compositeKey, expectedOld, newStoredValue));
305+
306+
if (!installed) {
307+
return null;
308+
}
309+
310+
deferred.run();
311+
try {
312+
return requireNonNull(deferred.getOrThrow());
313+
}
314+
catch (Throwable t) {
315+
this.storedValues.remove(compositeKey, newStoredValue);
316+
throw t;
317+
}
266318
}
267319

268320
/**
@@ -445,15 +497,8 @@ private record StoredValue(int order, Supplier<@Nullable Object> supplier) {
445497
return this.supplier.get();
446498
}
447499

448-
private @Nullable Object evaluateOrNullOnFailure() {
449-
if (this.supplier instanceof MemoizingSupplier memoizing) {
450-
return memoizing.getOrNullOnFailure();
451-
}
452-
return this.supplier.get();
453-
}
454-
455500
static @Nullable Object evaluateIfNotNull(@Nullable StoredValue value) {
456-
return value != null ? value.evaluateOrNullOnFailure() : null;
501+
return value != null ? value.evaluate() : null;
457502
}
458503

459504
}
@@ -471,6 +516,66 @@ private void close(CloseAction<N> closeAction) throws Throwable {
471516

472517
}
473518

519+
/**
520+
* Deferred computation that can be installed into the store without executing
521+
* user code while holding internal map locks.
522+
*
523+
* <p>For {@link #get(Object, Object)}, failures are treated as logically absent
524+
* (returning {@code null}) so exceptions are not observable via {@code get()}.
525+
*
526+
* <p>For {@link #computeIfAbsent(Object, Object, Function)},
527+
* {@link #getOrThrow()} rethrows the original failure.
528+
*/
529+
private static final class DeferredSupplier implements Supplier<@Nullable Object> {
530+
531+
private final FutureTask<@Nullable Object> task;
532+
533+
private DeferredSupplier(Supplier<@Nullable Object> delegate) {
534+
this.task = new FutureTask<>(delegate::get);
535+
}
536+
537+
private void run() {
538+
this.task.run();
539+
}
540+
541+
@Override
542+
public @Nullable Object get() {
543+
try {
544+
return this.task.get();
545+
}
546+
catch (InterruptedException e) {
547+
Thread.currentThread().interrupt();
548+
throw throwAsUncheckedException(e);
549+
}
550+
catch (ExecutionException e) {
551+
Throwable t = e.getCause();
552+
if (t == null) {
553+
t = e;
554+
}
555+
UnrecoverableExceptions.rethrowIfUnrecoverable(t);
556+
return null;
557+
}
558+
}
559+
560+
private @Nullable Object getOrThrow() {
561+
try {
562+
return this.task.get();
563+
}
564+
catch (InterruptedException e) {
565+
Thread.currentThread().interrupt();
566+
throw throwAsUncheckedException(e);
567+
}
568+
catch (ExecutionException e) {
569+
Throwable t = e.getCause();
570+
if (t == null) {
571+
t = e;
572+
}
573+
UnrecoverableExceptions.rethrowIfUnrecoverable(t);
574+
throw throwAsUncheckedException(t);
575+
}
576+
}
577+
}
578+
474579
/**
475580
* Thread-safe {@link Supplier} that memoizes the result of calling its
476581
* delegate and ensures it is called at most once.
@@ -485,18 +590,12 @@ private static class MemoizingSupplier implements Supplier<@Nullable Object> {
485590
private static final Object NO_VALUE_SET = new Object();
486591

487592
private final Supplier<@Nullable Object> delegate;
488-
private final boolean transientFailures;
489593

490594
@Nullable
491595
private volatile Object value = NO_VALUE_SET;
492596

493597
private MemoizingSupplier(Supplier<@Nullable Object> delegate) {
494-
this(delegate, false);
495-
}
496-
497-
private MemoizingSupplier(Supplier<@Nullable Object> delegate, boolean transientFailures) {
498598
this.delegate = delegate;
499-
this.transientFailures = transientFailures;
500599
}
501600

502601
@Override
@@ -510,19 +609,6 @@ private MemoizingSupplier(Supplier<@Nullable Object> delegate, boolean transient
510609
return this.value;
511610
}
512611

513-
private @Nullable Object getOrNullOnFailure() {
514-
if (this.value == NO_VALUE_SET) {
515-
computeValue();
516-
}
517-
if (this.value instanceof Failure failure) {
518-
if (this.transientFailures) {
519-
return null;
520-
}
521-
throw throwAsUncheckedException(failure.throwable);
522-
}
523-
return this.value;
524-
}
525-
526612
private synchronized void computeValue() {
527613
try {
528614
if (this.value == NO_VALUE_SET) {

0 commit comments

Comments
 (0)