Fix concurrency bug in NamespacedHierarchicalStore.computeIfAbsent

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,26 +237,30 @@ 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");
 		CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
 		StoredValue storedValue = getStoredValue(compositeKey);
 		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(() -> {
+			StoredValue newStoredValue = storedValues.compute(compositeKey, (__, oldStoredValue) -> {
+				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) {
+				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() {