diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/CompositeLock.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/CompositeLock.java index 3718d82a64fc..b09f1236f570 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/CompositeLock.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/CompositeLock.java @@ -15,6 +15,8 @@ import java.util.concurrent.ForkJoinPool; import java.util.concurrent.locks.Lock; +import org.junit.platform.commons.util.Preconditions; + /** * @since 1.3 */ @@ -23,7 +25,7 @@ class CompositeLock implements ResourceLock { private final List locks; CompositeLock(List locks) { - this.locks = locks; + this.locks = Preconditions.notEmpty(locks, "Locks must not be empty"); } // for tests only diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorService.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorService.java index 09afd8df3eb7..9b5d2efea14c 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorService.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorService.java @@ -16,6 +16,8 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.lang.reflect.Constructor; +import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Deque; import java.util.LinkedList; import java.util.List; @@ -26,7 +28,6 @@ import java.util.concurrent.ForkJoinTask; import java.util.concurrent.ForkJoinWorkerThread; import java.util.concurrent.Future; -import java.util.concurrent.RecursiveAction; import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.function.Predicate; @@ -51,7 +52,9 @@ public class ForkJoinPoolHierarchicalTestExecutorService implements HierarchicalTestExecutorService { private final ForkJoinPool forkJoinPool; + private final TaskEventListener taskEventListener; private final int parallelism; + private final ThreadLocal threadLocks = ThreadLocal.withInitial(ThreadLock::new); /** * Create a new {@code ForkJoinPoolHierarchicalTestExecutorService} based on @@ -71,7 +74,13 @@ public ForkJoinPoolHierarchicalTestExecutorService(ConfigurationParameters confi */ @API(status = STABLE, since = "1.10") public ForkJoinPoolHierarchicalTestExecutorService(ParallelExecutionConfiguration configuration) { + this(configuration, TaskEventListener.NOOP); + } + + ForkJoinPoolHierarchicalTestExecutorService(ParallelExecutionConfiguration configuration, + TaskEventListener taskEventListener) { forkJoinPool = createForkJoinPool(configuration); + this.taskEventListener = taskEventListener; parallelism = forkJoinPool.getParallelism(); LoggerFactory.getLogger(getClass()).config(() -> "Using ForkJoinPool with parallelism of " + parallelism); } @@ -132,7 +141,7 @@ public Future submit(TestTask testTask) { if (testTask.getExecutionMode() == CONCURRENT && ForkJoinTask.getSurplusQueuedTaskCount() < parallelism) { return exclusiveTask.fork(); } - exclusiveTask.compute(); + exclusiveTask.execSync(); return completedFuture(null); } @@ -143,7 +152,7 @@ private boolean isAlreadyRunningInForkJoinPool() { @Override public void invokeAll(List tasks) { if (tasks.size() == 1) { - new ExclusiveTask(tasks.get(0)).compute(); + new ExclusiveTask(tasks.get(0)).execSync(); return; } Deque nonConcurrentTasks = new LinkedList<>(); @@ -169,7 +178,7 @@ private void forkConcurrentTasks(List tasks, Deque nonConcurrentTasks) { for (ExclusiveTask task : nonConcurrentTasks) { - task.compute(); + task.execSync(); } } @@ -177,7 +186,18 @@ private void joinConcurrentTasksInReverseOrderToEnableWorkStealing( Deque concurrentTasksInReverseOrder) { for (ExclusiveTask forkedTask : concurrentTasksInReverseOrder) { forkedTask.join(); + resubmitDeferredTasks(); + } + } + + private void resubmitDeferredTasks() { + List deferredTasks = threadLocks.get().deferredTasks; + for (ExclusiveTask deferredTask : deferredTasks) { + if (!deferredTask.isDone()) { + deferredTask.fork(); + } } + deferredTasks.clear(); } @Override @@ -186,8 +206,8 @@ public void close() { } // this class cannot not be serialized because TestTask is not Serializable - @SuppressWarnings("serial") - static class ExclusiveTask extends RecursiveAction { + @SuppressWarnings({ "serial", "RedundantSuppression" }) + class ExclusiveTask extends ForkJoinTask { private final TestTask testTask; @@ -195,17 +215,61 @@ static class ExclusiveTask extends RecursiveAction { this.testTask = testTask; } + /** + * Always returns {@code null}. + * + * @return {@code null} always + */ + public final Void getRawResult() { + return null; + } + + /** + * Requires null completion value. + */ + protected final void setRawResult(Void mustBeNull) { + } + + void execSync() { + boolean completed = exec(); + if (!completed) { + throw new IllegalStateException( + "Task was deferred but should have been executed synchronously: " + testTask); + } + } + @SuppressWarnings("try") @Override - public void compute() { - try (ResourceLock lock = testTask.getResourceLock().acquire()) { + public boolean exec() { + // Check if this task is compatible with the current resource lock, if there is any. + // If not, we put this task in the thread local as a deferred task + // and let the worker thread fork it once it is done with the current task. + ResourceLock resourceLock = testTask.getResourceLock(); + ThreadLock threadLock = threadLocks.get(); + if (!threadLock.areAllHeldLocksCompatibleWith(resourceLock)) { + threadLock.addDeferredTask(this); + taskEventListener.deferred(testTask); + // Return false to indicate that this task is not done yet + // this means that .join() will wait. + return false; + } + try (ResourceLock lock = resourceLock.acquire()) { + threadLock.incrementNesting(lock); testTask.execute(); + return true; } catch (InterruptedException e) { throw ExceptionUtils.throwAsUncheckedException(e); } + finally { + threadLock.decrementNesting(); + } } + @Override + public String toString() { + return "ExclusiveTask [" + testTask + "]"; + } } static class WorkerThreadFactory implements ForkJoinPool.ForkJoinWorkerThreadFactory { @@ -228,4 +292,34 @@ static class WorkerThread extends ForkJoinWorkerThread { } + static class ThreadLock { + private final Deque locks = new ArrayDeque<>(2); + private final List deferredTasks = new ArrayList<>(); + + void addDeferredTask(ExclusiveTask task) { + deferredTasks.add(task); + } + + void incrementNesting(ResourceLock lock) { + locks.push(lock); + } + + @SuppressWarnings("resource") + void decrementNesting() { + locks.pop(); + } + + boolean areAllHeldLocksCompatibleWith(ResourceLock lock) { + return locks.stream().allMatch(l -> l.isCompatible(lock)); + } + } + + interface TaskEventListener { + + TaskEventListener NOOP = __ -> { + }; + + void deferred(TestTask testTask); + } + } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java index 223f7873525d..be997dc5715a 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java @@ -10,12 +10,16 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static java.util.Comparator.comparing; import static java.util.Comparator.naturalOrder; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toList; import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE; import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode.READ; import java.util.Collection; @@ -28,6 +32,9 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.junit.platform.engine.support.hierarchical.SingleLock.GlobalReadLock; +import org.junit.platform.engine.support.hierarchical.SingleLock.GlobalReadWriteLock; + /** * @since 1.3 */ @@ -42,20 +49,29 @@ private static Comparator globalKeyFirst() { } private final Map locksByKey = new ConcurrentHashMap<>(); + private final GlobalReadLock globalReadLock; + private final GlobalReadWriteLock globalReadWriteLock; + + public LockManager() { + globalReadLock = new GlobalReadLock(toLock(GLOBAL_READ)); + globalReadWriteLock = new GlobalReadWriteLock(toLock(GLOBAL_READ_WRITE)); + } ResourceLock getLockForResources(Collection resources) { - if (resources.size() == 1) { - return getLockForResource(getOnlyElement(resources)); - } - List locks = getDistinctSortedLocks(resources); - return toResourceLock(locks); + return toResourceLock(toDistinctSortedLocks(resources)); } ResourceLock getLockForResource(ExclusiveResource resource) { - return new SingleLock(toLock(resource)); + return toResourceLock(toLock(resource)); } - private List getDistinctSortedLocks(Collection resources) { + private List toDistinctSortedLocks(Collection resources) { + if (resources.isEmpty()) { + return emptyList(); + } + if (resources.size() == 1) { + return singletonList(toLock(getOnlyElement(resources))); + } // @formatter:off Map> resourcesByKey = resources.stream() .sorted(COMPARATOR) @@ -79,10 +95,20 @@ private ResourceLock toResourceLock(List locks) { case 0: return NopLock.INSTANCE; case 1: - return new SingleLock(locks.get(0)); + return toResourceLock(locks.get(0)); default: return new CompositeLock(locks); } } + private ResourceLock toResourceLock(Lock lock) { + if (lock == toLock(GLOBAL_READ)) { + return globalReadLock; + } + if (lock == toLock(GLOBAL_READ_WRITE)) { + return globalReadWriteLock; + } + return new SingleLock(lock); + } + } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTestTask.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTestTask.java index e4d2208f2bc3..8e602b990ba9 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTestTask.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTestTask.java @@ -79,6 +79,11 @@ public ExecutionMode getExecutionMode() { return taskContext.getExecutionAdvisor().getForcedExecutionMode(testDescriptor).orElse(node.getExecutionMode()); } + @Override + public String toString() { + return "NodeTestTask [" + testDescriptor + "]"; + } + void setParentContext(C parentContext) { this.parentContext = parentContext; } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java index 1dcdb5601881..19f6b4da061b 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java @@ -43,4 +43,12 @@ default void close() { release(); } + /** + * {@return whether the given lock is compatible with this lock} + * @param other the other lock to check for compatibility + */ + default boolean isCompatible(ResourceLock other) { + return this instanceof NopLock || other instanceof NopLock; + } + } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleLock.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleLock.java index e568b9a8f5c0..f3853dce4ee8 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleLock.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleLock.java @@ -60,4 +60,20 @@ public boolean isReleasable() { } + static class GlobalReadLock extends SingleLock { + GlobalReadLock(Lock lock) { + super(lock); + } + + @Override + public boolean isCompatible(ResourceLock other) { + return !(other instanceof GlobalReadWriteLock); + } + } + + static class GlobalReadWriteLock extends SingleLock { + GlobalReadWriteLock(Lock lock) { + super(lock); + } + } } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinDeadLockTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinDeadLockTests.java new file mode 100644 index 000000000000..b50b2e23c700 --- /dev/null +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinDeadLockTests.java @@ -0,0 +1,179 @@ +/* + * Copyright 2015-2024 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.platform.engine.support.hierarchical; + +import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT; +import static org.junit.jupiter.api.parallel.ExecutionMode.SAME_THREAD; +import static org.junit.jupiter.api.parallel.ResourceAccessMode.READ; +import static org.junit.jupiter.api.parallel.ResourceAccessMode.READ_WRITE; +import static org.junit.jupiter.api.parallel.Resources.SYSTEM_PROPERTIES; + +import java.time.LocalTime; +import java.util.Arrays; +import java.util.concurrent.CountDownLatch; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterTestExecutionCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.Isolated; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.engine.Constants; +import org.junit.platform.engine.discovery.ClassSelector; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.testkit.engine.EngineTestKit; + +// https://github.com/junit-team/junit5/issues/3945 +@Timeout(10) +public class ForkJoinDeadLockTests { + + @Test + void forkJoinExecutionDoesNotLeadToDeadLock() { + run(NonIsolatedTestCase.class, IsolatedTestCase.class, Isolated2TestCase.class); + } + + @Test + void nestedResourceLocksShouldStillWork() { + run(SharedResourceTestCase.class); + } + + @Test + void multiLevelLocks() { + run(ClassLevelTestCase.class); + } + + private static void run(Class... classes) { + EngineTestKit.engine("junit-jupiter") // + .selectors(Arrays.stream(classes).map(DiscoverySelectors::selectClass).toArray(ClassSelector[]::new)) // + .configurationParameter(Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, "true") // + .configurationParameter(Constants.DEFAULT_PARALLEL_EXECUTION_MODE, "concurrent") // + .configurationParameter(Constants.DEFAULT_CLASSES_EXECUTION_MODE_PROPERTY_NAME, "concurrent") // + .configurationParameter(Constants.PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, "fixed") // + .configurationParameter(Constants.PARALLEL_CONFIG_FIXED_MAX_POOL_SIZE_PROPERTY_NAME, "3") // + .configurationParameter(Constants.PARALLEL_CONFIG_FIXED_PARALLELISM_PROPERTY_NAME, "3") // + .configurationParameter(Constants.PARALLEL_CONFIG_FIXED_SATURATE_PROPERTY_NAME, "false") // + .execute(); + } + + @ExtendWith(StartFinishLogger.class) + static class BaseTestCase { + } + + @SuppressWarnings("JUnitMalformedDeclaration") + @Execution(CONCURRENT) + public static class NonIsolatedTestCase extends BaseTestCase { + + public static CountDownLatch otherThreadRunning = new CountDownLatch(1); + public static CountDownLatch sameThreadFinishing = new CountDownLatch(1); + + @Test + @Execution(CONCURRENT) + void otherThread() throws Exception { + otherThreadRunning.countDown(); + sameThreadFinishing.await(); + Thread.sleep(100); + } + + @Test + @Execution(SAME_THREAD) + void sameThread() throws Exception { + otherThreadRunning.await(); + sameThreadFinishing.countDown(); + } + } + + @SuppressWarnings("JUnitMalformedDeclaration") + @Isolated + public static class IsolatedTestCase extends BaseTestCase { + + @Test + void test() throws Exception { + Thread.sleep(100); + } + } + + static class Isolated2TestCase extends IsolatedTestCase { + } + + @SuppressWarnings("JUnitMalformedDeclaration") + public static class SharedResourceTestCase { + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ) + void customPropertyIsNotSetByDefault() { + } + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ_WRITE) + void canSetCustomPropertyToApple() { + } + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ_WRITE) + void canSetCustomPropertyToBanana() { + } + } + + @SuppressWarnings("JUnitMalformedDeclaration") + @ResourceLock(value = "foo", mode = READ_WRITE) + public static class ClassLevelTestCase { + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ) + void customPropertyIsNotSetByDefault() { + } + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ_WRITE) + void canSetCustomPropertyToApple() { + } + + @Test + @ResourceLock(value = SYSTEM_PROPERTIES, mode = READ_WRITE) + void canSetCustomPropertyToBanana() { + } + } + + static class StartFinishLogger + implements BeforeTestExecutionCallback, AfterTestExecutionCallback, BeforeAllCallback, AfterAllCallback { + + @Override + public void beforeAll(ExtensionContext context) { + log("starting class " + context.getTestClass().orElseThrow().getSimpleName()); + } + + @Override + public void beforeTestExecution(ExtensionContext context) { + log("starting method " + context.getTestClass().orElseThrow().getSimpleName() + "." + + context.getTestMethod().orElseThrow().getName()); + } + + @Override + public void afterTestExecution(ExtensionContext context) { + log("finishing method " + context.getTestClass().orElseThrow().getSimpleName() + "." + + context.getTestMethod().orElseThrow().getName()); + } + + @Override + public void afterAll(ExtensionContext context) { + log("finishing class " + context.getTestClass().orElseThrow().getSimpleName()); + } + } + + private static void log(String message) { + System.out.println("[" + LocalTime.now() + "] " + Thread.currentThread().getName() + " - " + message); + } +} diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorServiceTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorServiceTests.java index 79120bf40d17..f3ee3d923ab6 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorServiceTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorServiceTests.java @@ -11,11 +11,26 @@ package org.junit.platform.engine.support.hierarchical; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE; +import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.CONCURRENT; + +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.function.ThrowingConsumer; import org.junit.platform.commons.JUnitException; +import org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.TaskEventListener; +import org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService.TestTask; +import org.junit.platform.engine.support.hierarchical.Node.ExecutionMode; class ForkJoinPoolHierarchicalTestExecutorServiceTests { @@ -33,4 +48,86 @@ void exceptionsFromInvalidConfigurationAreNotSwallowed() { assertThat(exception).rootCause().isInstanceOf(IllegalArgumentException.class); } + @Test + @Timeout(5) + void defersTasksWithIncompatibleLocks() throws Exception { + var configuration = new DefaultParallelExecutionConfiguration(2, 2, 2, 2, 1, __ -> true); + + var lockManager = new LockManager(); + var globalReadLock = lockManager.getLockForResource(GLOBAL_READ); + var globalReadWriteLock = lockManager.getLockForResource(GLOBAL_READ_WRITE); + var nopLock = NopLock.INSTANCE; + + var threadNamesByTaskIdentifier = new ConcurrentHashMap(); + var deferred = new CountDownLatch(1); + var deferredTask = new AtomicReference(); + + TaskEventListener taskEventListener = testTask -> { + deferredTask.set(testTask); + deferred.countDown(); + }; + + var isolatedTask = new DummyTestTask("isolatedTask", globalReadWriteLock, + t -> threadNamesByTaskIdentifier.put(t.identifier(), Thread.currentThread().getName())); + + try (var pool = new ForkJoinPoolHierarchicalTestExecutorService(configuration, taskEventListener)) { + + var bothLeafTasksAreRunning = new CountDownLatch(2); + var nestedTask = new DummyTestTask("nestedTask", globalReadLock, t -> { + threadNamesByTaskIdentifier.put(t.identifier(), Thread.currentThread().getName()); + var leafTask1 = new DummyTestTask("leafTask1", nopLock, t1 -> { + threadNamesByTaskIdentifier.put(t1.identifier(), Thread.currentThread().getName()); + pool.new ExclusiveTask(isolatedTask).fork(); + bothLeafTasksAreRunning.countDown(); + bothLeafTasksAreRunning.await(); + try { + deferred.await(); + } + catch (InterruptedException e) { + System.out.println("Interrupted while waiting for task to be deferred"); + } + }); + var leafTask2 = new DummyTestTask("leafTask2", nopLock, t2 -> { + threadNamesByTaskIdentifier.put(t2.identifier(), Thread.currentThread().getName()); + bothLeafTasksAreRunning.countDown(); + bothLeafTasksAreRunning.await(); + }); + pool.invokeAll(List.of(leafTask1, leafTask2)); + }); + + pool.submit(nestedTask).get(); + } + + assertEquals(isolatedTask, deferredTask.get()); + assertEquals(threadNamesByTaskIdentifier.get("nestedTask"), threadNamesByTaskIdentifier.get("leafTask2")); + assertNotEquals(threadNamesByTaskIdentifier.get("leafTask1"), threadNamesByTaskIdentifier.get("leafTask2")); + } + + record DummyTestTask(String identifier, ResourceLock resourceLock, ThrowingConsumer action) + implements TestTask { + @Override + public ExecutionMode getExecutionMode() { + return CONCURRENT; + } + + @Override + public ResourceLock getResourceLock() { + return resourceLock; + } + + @Override + public void execute() { + try { + action.accept(this); + } + catch (Throwable e) { + throw new RuntimeException("Action " + identifier + " failed", e); + } + } + + @Override + public String toString() { + return identifier; + } + } } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/LockManagerTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/LockManagerTests.java index 5fca505147f1..c9cb0570027e 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/LockManagerTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/LockManagerTests.java @@ -32,7 +32,7 @@ */ class LockManagerTests { - private LockManager lockManager = new LockManager(); + private final LockManager lockManager = new LockManager(); @Test void returnsNopLockWithoutExclusiveResources() { @@ -50,7 +50,7 @@ void returnsSingleLockForSingleExclusiveResource() { var locks = getLocks(resources, SingleLock.class); assertThat(locks).hasSize(1); - assertThat(locks.get(0)).isInstanceOf(ReadLock.class); + assertThat(locks.getFirst()).isInstanceOf(ReadLock.class); } @Test @@ -75,7 +75,7 @@ void reusesSameLockForExclusiveResourceWithSameKey() { assertThat(locks1).hasSize(1); assertThat(locks2).hasSize(1); - assertThat(locks1.get(0)).isSameAs(locks2.get(0)); + assertThat(locks1.getFirst()).isSameAs(locks2.getFirst()); } @Test @@ -111,8 +111,26 @@ void globalLockComesFirst(LockMode globalLockMode) { assertThat(locks.get(3)).isEqualTo(getSingleLock("foo", READ_WRITE)); } - private Lock getSingleLock(String globalResourceLockKey, LockMode read) { - return getLocks(Set.of(new ExclusiveResource(globalResourceLockKey, read)), SingleLock.class).get(0); + @Test + void usesSpecialClassForGlobalReadLock() { + var lock = lockManager.getLockForResources(List.of(ExclusiveResource.GLOBAL_READ)); + + assertThat(lock) // + .isInstanceOf(SingleLock.GlobalReadLock.class) // + .isSameAs(lockManager.getLockForResource(ExclusiveResource.GLOBAL_READ)); + } + + @Test + void usesSpecialClassForGlobalReadWriteLock() { + var lock = lockManager.getLockForResources(List.of(ExclusiveResource.GLOBAL_READ_WRITE)); + + assertThat(lock) // + .isInstanceOf(SingleLock.GlobalReadWriteLock.class) // + .isSameAs(lockManager.getLockForResource(ExclusiveResource.GLOBAL_READ_WRITE)); + } + + private Lock getSingleLock(String key, LockMode lockMode) { + return getLocks(Set.of(new ExclusiveResource(key, lockMode)), SingleLock.class).getFirst(); } private List getLocks(Collection resources, Class type) { diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ResourceLockTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ResourceLockTests.java new file mode 100644 index 000000000000..ca80c33d6131 --- /dev/null +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ResourceLockTests.java @@ -0,0 +1,82 @@ +/* + * Copyright 2015-2024 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.platform.engine.support.hierarchical; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.concurrent.locks.ReentrantLock; + +import org.junit.jupiter.api.Test; + +@SuppressWarnings("resource") +class ResourceLockTests { + + @Test + void nopLocksAreCompatibleWithEverything() { + var nopLock = NopLock.INSTANCE; + + assertTrue(nopLock.isCompatible(NopLock.INSTANCE)); + assertTrue(nopLock.isCompatible(new SingleLock(anyReentrantLock()))); + assertTrue(nopLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock()))); + assertTrue(nopLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock()))); + assertTrue(nopLock.isCompatible(new CompositeLock(List.of(anyReentrantLock())))); + } + + @Test + void singleLocksAreIncompatibleWithNonNopLocks() { + var singleLock = new SingleLock(anyReentrantLock()); + + assertTrue(singleLock.isCompatible(NopLock.INSTANCE)); + assertFalse(singleLock.isCompatible(new SingleLock(anyReentrantLock()))); + assertFalse(singleLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock()))); + assertFalse(singleLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock()))); + assertFalse(singleLock.isCompatible(new CompositeLock(List.of(anyReentrantLock())))); + } + + @Test + void globalReadLockIsCompatibleWithEverythingExceptGlobalReadWriteLock() { + var globalReadLock = new SingleLock.GlobalReadLock(anyReentrantLock()); + + assertTrue(globalReadLock.isCompatible(NopLock.INSTANCE)); + assertTrue(globalReadLock.isCompatible(new SingleLock(anyReentrantLock()))); + assertTrue(globalReadLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock()))); + assertFalse(globalReadLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock()))); + assertTrue(globalReadLock.isCompatible(new CompositeLock(List.of(anyReentrantLock())))); + } + + @Test + void globalReadWriteLockIsIncompatibleWithWithNonNopLocks() { + var globalReadWriteLock = new SingleLock.GlobalReadWriteLock(anyReentrantLock()); + + assertTrue(globalReadWriteLock.isCompatible(NopLock.INSTANCE)); + assertFalse(globalReadWriteLock.isCompatible(new SingleLock(anyReentrantLock()))); + assertFalse(globalReadWriteLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock()))); + assertFalse(globalReadWriteLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock()))); + assertFalse(globalReadWriteLock.isCompatible(new CompositeLock(List.of(anyReentrantLock())))); + } + + @Test + void compositeLocksAreIncompatibleWithNonNopLocks() { + CompositeLock compositeLock = new CompositeLock(List.of(anyReentrantLock())); + + assertTrue(compositeLock.isCompatible(NopLock.INSTANCE)); + assertFalse(compositeLock.isCompatible(new SingleLock(anyReentrantLock()))); + assertFalse(compositeLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock()))); + assertFalse(compositeLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock()))); + assertFalse(compositeLock.isCompatible(compositeLock)); + } + + private static ReentrantLock anyReentrantLock() { + return new ReentrantLock(); + } +}