Fix concurrency bug in NamespacedHierarchicalStore.computeIfAbsent

`computeIfAbsent` previously invoked `defaultCreator` while holding the
store's internal map lock. Under parallel execution this could cause
threads using the store to block each other and temporarily see missing
or incorrectly initialized state for values created via
`computeIfAbsent`.

The implementation now wraps `defaultCreator` in a `MemoizingSupplier`
and installs that supplier via the map operation, evaluating it only
after the update has completed. This avoids running user code while
holding the lock and aligns `computeIfAbsent`'s behavior with the
deprecated `getOrComputeIfAbsent`, preserving the intended "one
initialization per key" semantics.

Issue: #5171
Signed-off-by: martinfrancois <f.martin@fastmail.com>

Author: martinfrancois

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

@@ -17,6 +17,13 @@ repository on GitHub.
 ==== Bug Fixes
 
 * Make `ConsoleLauncher` compatible with JDK 26 by avoiding final field mutations.
+* Fix a concurrency bug in `NamespacedHierarchicalStore.computeIfAbsent` where
+  the `defaultCreator` function was executed while holding the store's internal
+  map lock. Under parallel execution, this could cause threads using the store to
+  block each other and temporarily see a missing or incorrectly initialized state
+  for values created via `computeIfAbsent`. The method now evaluates
+  `defaultCreator` outside the critical section using a memoizing supplier,
+  aligning its behavior with the deprecated `getOrComputeIfAbsent`.
 
 [[v6.0.2-junit-platform-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes

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

@@ -237,6 +237,7 @@ public void close() {
 	 * closed
 	 * @since 6.0
 	 */
+	@SuppressWarnings("ReferenceEquality")
 	@API(status = MAINTAINED, since = "6.0")
 	public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? extends V> defaultCreator) {
 		Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
@@ -245,18 +246,21 @@ public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? e
 		var result = StoredValue.evaluateIfNotNull(storedValue);
 		if (result == null) {
 			StoredValue newStoredValue = this.storedValues.compute(compositeKey, (__, oldStoredValue) -> {
-				if (StoredValue.evaluateIfNotNull(oldStoredValue) == null) {
-					rejectIfClosed();
-					var computedValue = Preconditions.notNull(defaultCreator.apply(key),
-						"defaultCreator must not return null");
-					return newStoredValue(() -> {
+				if (oldStoredValue == null || oldStoredValue == storedValue) {
+					return newStoredValue(new MemoizingSupplier(() -> {
 						rejectIfClosed();
-						return computedValue;
-					});
+						return Preconditions.notNull(defaultCreator.apply(key), "defaultCreator must not return null");
+					}));
 				}
 				return oldStoredValue;
 			});
-			return requireNonNull(newStoredValue.evaluate());
+			try {
+				return requireNonNull(newStoredValue.evaluate());
+			}
+			catch (Throwable t) {
+				this.storedValues.remove(compositeKey, newStoredValue);
+				throw t;
+			}
 		}
 		return result;
 	}

platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java

@@ -26,6 +26,9 @@
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 
@@ -416,6 +419,115 @@ void simulateRaceConditionInComputeIfAbsent() throws Exception {
 			assertEquals(1, counter.get());
 			assertThat(values).hasSize(threads).containsOnly(1);
 		}
+
+		@SuppressWarnings("deprecation")
+		@Test
+		void getOrComputeIfAbsentDoesNotDeadlockWithCollidingKeys() throws Exception {
+			try (var localStore = new NamespacedHierarchicalStore<String>(null)) {
+				var firstComputationStarted = new CountDownLatch(1);
+				var secondComputationAllowedToFinish = new CountDownLatch(1);
+				var firstThreadTimedOut = new AtomicBoolean(false);
+
+				Thread first = new Thread(
+					() -> localStore.getOrComputeIfAbsent(namespace, new CollidingKey("k1"), __ -> {
+						firstComputationStarted.countDown();
+						try {
+							if (!secondComputationAllowedToFinish.await(200, TimeUnit.MILLISECONDS)) {
+								firstThreadTimedOut.set(true);
+							}
+						}
+						catch (InterruptedException e) {
+							Thread.currentThread().interrupt();
+						}
+						return "value1";
+					}));
+
+				Thread second = new Thread(() -> {
+					try {
+						firstComputationStarted.await();
+					}
+					catch (InterruptedException e) {
+						Thread.currentThread().interrupt();
+					}
+					localStore.getOrComputeIfAbsent(namespace, new CollidingKey("k2"), __ -> {
+						secondComputationAllowedToFinish.countDown();
+						return "value2";
+					});
+				});
+
+				first.start();
+				second.start();
+
+				first.join(1000);
+				second.join(1000);
+
+				assertThat(firstThreadTimedOut).as(
+					"getOrComputeIfAbsent should not block subsequent computations on colliding keys").isFalse();
+			}
+		}
+
+		@Test
+		void computeIfAbsentCanDeadlockWithCollidingKeys() throws Exception {
+			try (var localStore = new NamespacedHierarchicalStore<String>(null)) {
+				var firstComputationStarted = new CountDownLatch(1);
+				var secondComputationAllowedToFinish = new CountDownLatch(1);
+				var firstThreadTimedOut = new AtomicBoolean(false);
+
+				Thread first = new Thread(() -> localStore.computeIfAbsent(namespace, new CollidingKey("k1"), __ -> {
+					firstComputationStarted.countDown();
+					try {
+						if (!secondComputationAllowedToFinish.await(200, TimeUnit.MILLISECONDS)) {
+							firstThreadTimedOut.set(true);
+						}
+					}
+					catch (InterruptedException e) {
+						Thread.currentThread().interrupt();
+					}
+					return "value1";
+				}));
+
+				Thread second = new Thread(() -> {
+					try {
+						firstComputationStarted.await();
+					}
+					catch (InterruptedException e) {
+						Thread.currentThread().interrupt();
+					}
+					localStore.computeIfAbsent(namespace, new CollidingKey("k2"), __ -> {
+						secondComputationAllowedToFinish.countDown();
+						return "value2";
+					});
+				});
+
+				first.start();
+				second.start();
+
+				first.join(1000);
+				second.join(1000);
+
+				assertThat(firstThreadTimedOut).as(
+					"computeIfAbsent should not block subsequent computations on colliding keys").isFalse();
+			}
+		}
+
+		@Test
+		void computeIfAbsentOverridesParentNullValue() {
+			// computeIfAbsent must treat a null value from the parent store as logically absent,
+			// so the child store can install and keep its own non-null value for the same key.
+			try (var parent = new NamespacedHierarchicalStore<String>(null);
+					var child = new NamespacedHierarchicalStore<String>(parent)) {
+
+				parent.put(namespace, key, null);
+
+				assertNull(parent.get(namespace, key));
+				assertNull(child.get(namespace, key));
+
+				Object childValue = child.computeIfAbsent(namespace, key, __ -> "value");
+
+				assertEquals("value", childValue);
+				assertEquals("value", child.get(namespace, key));
+			}
+		}
 	}
 
 	@Nested
@@ -663,6 +775,36 @@ private void assertClosed() {
 
 	}
 
+	private static final class CollidingKey {
+
+		private final String value;
+
+		private CollidingKey(String value) {
+			this.value = value;
+		}
+
+		@Override
+		public int hashCode() {
+			return 42;
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if (this == obj) {
+				return true;
+			}
+			if (!(obj instanceof CollidingKey other)) {
+				return false;
+			}
+			return this.value.equals(other.value);
+		}
+
+		@Override
+		public String toString() {
+			return this.value;
+		}
+	}
+
 	private static Object createObject(String display) {
 		return new Object() {