Skip to content

Commit be7ab95

Browse files
committed
Only allow caller owning the DeferredSupplier to run it
1 parent 160a4ed commit be7ab95

File tree

1 file changed

+30
-5
lines changed

1 file changed

+30
-5
lines changed

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

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public void close() {
221221
}
222222
var newStoredValue = this.storedValues.compute(compositeKey, //
223223
(__, oldStoredValue) -> {
224-
if (isPresent(oldStoredValue)) {
224+
if (isStoredValuePresent(oldStoredValue)) {
225225
return oldStoredValue;
226226
}
227227
rejectIfClosed();
@@ -232,6 +232,7 @@ public void close() {
232232
});
233233

234234
if (newStoredValue instanceof StoredValue.DeferredValue value) {
235+
// Any thread that won the race may run the DeferredSupplier
235236
value.delegate().run();
236237
}
237238
return requireNonNull(newStoredValue.evaluate());
@@ -261,20 +262,22 @@ public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? e
261262
if (result != null) {
262263
return result;
263264
}
265+
Object ownerToken = new Object();
264266
StoredValue newStoredValue = this.storedValues.compute(compositeKey, (__, oldStoredValue) -> {
265267
if (StoredValue.evaluateIfNotNull(oldStoredValue) != null) {
266268
return oldStoredValue;
267269
}
268270
rejectIfClosed();
269-
return newStoredSuppliedValue(new DeferredSupplier(() -> {
271+
return newStoredSuppliedValue(new DeferredSupplier(ownerToken, () -> {
270272
rejectIfClosed();
271273
return Preconditions.notNull(defaultCreator.apply(key), "defaultCreator must not return null");
272274
}));
273275
});
274276

275277
if (newStoredValue instanceof StoredValue.DeferredOptionalValue value) {
276278
var delegate = value.delegate();
277-
delegate.run();
279+
// Only the thread that created the DeferredSupplier may run it
280+
delegate.run(ownerToken);
278281
return requireNonNull(delegate.getOrThrow());
279282
}
280283
// put or getOrComputeIfAbsent won the race
@@ -409,7 +412,7 @@ private StoredValue.DeferredOptionalValue newStoredSuppliedValue(DeferredSupplie
409412

410413
private @Nullable StoredValue getStoredValue(CompositeKey<N> compositeKey) {
411414
StoredValue storedValue = this.storedValues.get(compositeKey);
412-
if (isPresent(storedValue)) {
415+
if (isStoredValuePresent(storedValue)) {
413416
return storedValue;
414417
}
415418
if (this.parentStore != null) {
@@ -418,7 +421,7 @@ private StoredValue.DeferredOptionalValue newStoredSuppliedValue(DeferredSupplie
418421
return null;
419422
}
420423

421-
private static boolean isPresent(@Nullable StoredValue value) {
424+
private static boolean isStoredValuePresent(@Nullable StoredValue value) {
422425
return value != null && value.isPresent();
423426
}
424427

@@ -473,6 +476,9 @@ private interface StoredValue {
473476
return value != null ? value.evaluate() : null;
474477
}
475478

479+
/**
480+
* May contain {@code null} or a value, never an exception.
481+
*/
476482
record Value(int order, @Nullable Object value) implements StoredValue {
477483

478484
@Override
@@ -486,6 +492,9 @@ public boolean isPresent() {
486492
}
487493
}
488494

495+
/**
496+
* May eventually contain {@code null} or a value or an exception.
497+
*/
489498
record DeferredValue(int order, DeferredSupplier delegate) implements StoredValue {
490499

491500
@Override
@@ -499,6 +508,9 @@ public boolean isPresent() {
499508
}
500509
}
501510

511+
/**
512+
* May eventually contain a value or an exception, never {@code null}.
513+
*/
502514
record DeferredOptionalValue(int order, DeferredSupplier delegate) implements StoredValue {
503515

504516
@Override
@@ -548,12 +560,25 @@ private void close(CloseAction<N> closeAction) throws Throwable {
548560
private static final class DeferredSupplier implements Supplier<@Nullable Object> {
549561

550562
private final FutureTask<@Nullable Object> task;
563+
private final @Nullable Object ownerToken;
551564

552565
DeferredSupplier(Supplier<@Nullable Object> delegate) {
566+
this(null, delegate);
567+
}
568+
569+
DeferredSupplier(@Nullable Object ownerToken, Supplier<@Nullable Object> delegate) {
570+
this.ownerToken = ownerToken;
553571
this.task = new FutureTask<>(delegate::get);
554572
}
555573

556574
void run() {
575+
run(null);
576+
}
577+
578+
void run(@Nullable Object ownerToken) {
579+
if (!Objects.equals(this.ownerToken, ownerToken)) {
580+
return;
581+
}
557582
this.task.run();
558583
}
559584

0 commit comments

Comments
 (0)