From 6529d8d4b14e0ab2b9134a9d9b0d9260ba2f6410 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 21 Sep 2024 10:58:46 +0200 Subject: [PATCH] Allow for work stealing when only holding read locks (#4012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows tasks with only read locks to be stolen by threads that are currently holding only read locks. --------- Co-authored-by: Leonard Brünings --- .../support/hierarchical/CompositeLock.java | 28 +- .../hierarchical/ExclusiveResource.java | 11 + ...inPoolHierarchicalTestExecutorService.java | 12 +- .../support/hierarchical/LockManager.java | 67 ++-- .../engine/support/hierarchical/NopLock.java | 20 ++ .../support/hierarchical/ResourceLock.java | 45 ++- .../support/hierarchical/SingleLock.java | 44 +-- .../hierarchical/CompositeLockTests.java | 14 +- ...lHierarchicalTestExecutorServiceTests.java | 306 +++++++++++++++--- .../hierarchical/LockManagerTests.java | 8 +- .../hierarchical/ResourceLockTests.java | 146 ++++++--- .../support/hierarchical/SingleLockTests.java | 9 +- 12 files changed, 560 insertions(+), 150 deletions(-) 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 b09f1236f570..aff3111df90b 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 @@ -10,22 +10,36 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.Collections.unmodifiableList; + import java.util.ArrayList; import java.util.List; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.locks.Lock; import org.junit.platform.commons.util.Preconditions; +import org.junit.platform.commons.util.ToStringBuilder; /** * @since 1.3 */ class CompositeLock implements ResourceLock { + private final List resources; private final List locks; + private final boolean exclusive; - CompositeLock(List locks) { + CompositeLock(List resources, List locks) { + Preconditions.condition(resources.size() == locks.size(), "Resources and locks must have the same size"); + this.resources = unmodifiableList(resources); this.locks = Preconditions.notEmpty(locks, "Locks must not be empty"); + this.exclusive = resources.stream().anyMatch( + resource -> resource.getLockMode() == ExclusiveResource.LockMode.READ_WRITE); + } + + @Override + public List getResources() { + return resources; } // for tests only @@ -64,6 +78,18 @@ private void release(List acquiredLocks) { } } + @Override + public boolean isExclusive() { + return exclusive; + } + + @Override + public String toString() { + return new ToStringBuilder(this) // + .append("resources", resources) // + .toString(); + } + private class CompositeLockManagedBlocker implements ForkJoinPool.ManagedBlocker { private volatile boolean acquired; diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExclusiveResource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExclusiveResource.java index 4d090c3a69d9..621fe2ed211b 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExclusiveResource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExclusiveResource.java @@ -10,8 +10,11 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; import static org.apiguardian.api.API.Status.STABLE; +import java.util.Comparator; import java.util.Objects; import java.util.concurrent.locks.ReadWriteLock; @@ -50,6 +53,14 @@ public class ExclusiveResource { static final ExclusiveResource GLOBAL_READ = new ExclusiveResource(GLOBAL_KEY, LockMode.READ); static final ExclusiveResource GLOBAL_READ_WRITE = new ExclusiveResource(GLOBAL_KEY, LockMode.READ_WRITE); + static final Comparator COMPARATOR // + = comparing(ExclusiveResource::getKey, globalKeyFirst().thenComparing(naturalOrder())) // + .thenComparing(ExclusiveResource::getLockMode); + + private static Comparator globalKeyFirst() { + return comparing(key -> !GLOBAL_KEY.equals(key)); + } + private final String key; private final LockMode lockMode; private int hash; 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 c6ed2cb4b585..fc0aae08f68c 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 @@ -12,6 +12,7 @@ import static java.util.concurrent.CompletableFuture.completedFuture; import static org.apiguardian.api.API.Status.STABLE; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE; import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.CONCURRENT; import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.SAME_THREAD; @@ -39,7 +40,6 @@ import org.junit.platform.commons.logging.LoggerFactory; import org.junit.platform.commons.util.ExceptionUtils; import org.junit.platform.engine.ConfigurationParameters; -import org.junit.platform.engine.support.hierarchical.SingleLock.GlobalReadWriteLock; /** * A {@link ForkJoinPool}-based @@ -53,7 +53,9 @@ @API(status = STABLE, since = "1.10") public class ForkJoinPoolHierarchicalTestExecutorService implements HierarchicalTestExecutorService { - private final ForkJoinPool forkJoinPool; + // package-private for testing + final ForkJoinPool forkJoinPool; + private final TaskEventListener taskEventListener; private final int parallelism; private final ThreadLocal threadLocks = ThreadLocal.withInitial(ThreadLock::new); @@ -170,7 +172,7 @@ private void forkConcurrentTasks(List tasks, Deque sameThreadTasks, Deque concurrentTasksInReverseOrder) { for (TestTask testTask : tasks) { ExclusiveTask exclusiveTask = new ExclusiveTask(testTask); - if (testTask.getResourceLock() instanceof GlobalReadWriteLock) { + if (requiresGlobalReadWriteLock(testTask)) { isolatedTasks.add(exclusiveTask); } else if (testTask.getExecutionMode() == SAME_THREAD) { @@ -183,6 +185,10 @@ else if (testTask.getExecutionMode() == SAME_THREAD) { } } + private static boolean requiresGlobalReadWriteLock(TestTask testTask) { + return testTask.getResourceLock().getResources().contains(GLOBAL_READ_WRITE); + } + private void executeSync(Deque tasks) { for (ExclusiveTask task : tasks) { task.execSync(); 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 be997dc5715a..8e055cab7f36 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 @@ -12,18 +12,15 @@ 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.commons.util.CollectionUtils.toUnmodifiableList; 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; -import java.util.Comparator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -32,83 +29,75 @@ 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 */ class LockManager { - private static final Comparator COMPARATOR // - = comparing(ExclusiveResource::getKey, globalKeyFirst().thenComparing(naturalOrder())) // - .thenComparing(ExclusiveResource::getLockMode); - - private static Comparator globalKeyFirst() { - return comparing(key -> !GLOBAL_KEY.equals(key)); - } - private final Map locksByKey = new ConcurrentHashMap<>(); - private final GlobalReadLock globalReadLock; - private final GlobalReadWriteLock globalReadWriteLock; + private final SingleLock globalReadLock; + private final SingleLock globalReadWriteLock; public LockManager() { - globalReadLock = new GlobalReadLock(toLock(GLOBAL_READ)); - globalReadWriteLock = new GlobalReadWriteLock(toLock(GLOBAL_READ_WRITE)); + globalReadLock = new SingleLock(GLOBAL_READ, toLock(GLOBAL_READ)); + globalReadWriteLock = new SingleLock(GLOBAL_READ_WRITE, toLock(GLOBAL_READ_WRITE)); } ResourceLock getLockForResources(Collection resources) { - return toResourceLock(toDistinctSortedLocks(resources)); + return toResourceLock(toDistinctSortedResources(resources)); } ResourceLock getLockForResource(ExclusiveResource resource) { - return toResourceLock(toLock(resource)); + return toResourceLock(singletonList(resource)); } - private List toDistinctSortedLocks(Collection resources) { + private List toDistinctSortedResources(Collection resources) { if (resources.isEmpty()) { return emptyList(); } if (resources.size() == 1) { - return singletonList(toLock(getOnlyElement(resources))); + return singletonList(getOnlyElement(resources)); } // @formatter:off Map> resourcesByKey = resources.stream() - .sorted(COMPARATOR) + .sorted(ExclusiveResource.COMPARATOR) .distinct() .collect(groupingBy(ExclusiveResource::getKey, LinkedHashMap::new, toList())); return resourcesByKey.values().stream() .map(resourcesWithSameKey -> resourcesWithSameKey.get(0)) - .map(this::toLock) - .collect(toList()); + .collect(toUnmodifiableList()); // @formatter:on } - private Lock toLock(ExclusiveResource resource) { - ReadWriteLock lock = this.locksByKey.computeIfAbsent(resource.getKey(), key -> new ReentrantReadWriteLock()); - return resource.getLockMode() == READ ? lock.readLock() : lock.writeLock(); - } - - private ResourceLock toResourceLock(List locks) { - switch (locks.size()) { + private ResourceLock toResourceLock(List resources) { + switch (resources.size()) { case 0: return NopLock.INSTANCE; case 1: - return toResourceLock(locks.get(0)); + return toSingleLock(getOnlyElement(resources)); default: - return new CompositeLock(locks); + return new CompositeLock(resources, toLocks(resources)); } } - private ResourceLock toResourceLock(Lock lock) { - if (lock == toLock(GLOBAL_READ)) { + private SingleLock toSingleLock(ExclusiveResource resource) { + if (GLOBAL_READ.equals(resource)) { return globalReadLock; } - if (lock == toLock(GLOBAL_READ_WRITE)) { + if (GLOBAL_READ_WRITE.equals(resource)) { return globalReadWriteLock; } - return new SingleLock(lock); + return new SingleLock(resource, toLock(resource)); + } + + private List toLocks(List resources) { + return resources.stream().map(this::toLock).collect(toUnmodifiableList()); + } + + private Lock toLock(ExclusiveResource resource) { + ReadWriteLock lock = this.locksByKey.computeIfAbsent(resource.getKey(), key -> new ReentrantReadWriteLock()); + return resource.getLockMode() == READ ? lock.readLock() : lock.writeLock(); } } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NopLock.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NopLock.java index 6c885ecb0403..7cdd79592d53 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NopLock.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NopLock.java @@ -10,6 +10,12 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.Collections.emptyList; + +import java.util.List; + +import org.junit.platform.commons.util.ToStringBuilder; + /** * No-op {@link ResourceLock} implementation. * @@ -22,6 +28,11 @@ class NopLock implements ResourceLock { private NopLock() { } + @Override + public List getResources() { + return emptyList(); + } + @Override public ResourceLock acquire() { return this; @@ -32,4 +43,13 @@ public void release() { // nothing to do } + @Override + public boolean isExclusive() { + return false; + } + + @Override + public String toString() { + return new ToStringBuilder(this).toString(); + } } 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 19f6b4da061b..7e10c3ba7941 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 @@ -12,6 +12,9 @@ import static org.apiguardian.api.API.Status.STABLE; +import java.util.List; +import java.util.Optional; + import org.apiguardian.api.API; /** @@ -43,12 +46,50 @@ default void close() { release(); } + /** + * {@return the exclusive resources this lock represents} + */ + List getResources(); + + /** + * {@return whether this lock requires exclusiveness} + */ + boolean isExclusive(); + /** * {@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; - } + List ownResources = this.getResources(); + List otherResources = other.getResources(); + + if (ownResources.isEmpty() || otherResources.isEmpty()) { + return true; + } + + // Whenever there's a READ_WRITE lock, it's incompatible with any other lock + // because we guarantee that all children will have exclusive access to the + // resource in question. In practice, whenever a READ_WRITE lock is present, + // NodeTreeWalker will force all children to run in the same thread so that + // it should never attempt to steal work from another thread, and we shouldn't + // actually reach this point. + // The global read lock (which is always on direct children of the engine node) + // needs special treatment so that it is compatible with the first write lock + // (which may be on a test method). + boolean isGlobalReadLock = ownResources.size() == 1 + && ExclusiveResource.GLOBAL_READ.equals(ownResources.get(0)); + if ((!isGlobalReadLock && other.isExclusive()) || this.isExclusive()) { + return false; + } + + Optional potentiallyDeadlockCausingAdditionalResource = otherResources.stream() // + .filter(resource -> !ownResources.contains(resource)) // + .findFirst() // + .filter(resource -> ExclusiveResource.COMPARATOR.compare(resource, + ownResources.get(ownResources.size() - 1)) < 0); + + return !(potentiallyDeadlockCausingAdditionalResource.isPresent()); + } } 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 f3853dce4ee8..cfa9d2f3acf0 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 @@ -10,20 +10,33 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.Collections.singletonList; +import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; + +import java.util.List; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.locks.Lock; +import org.junit.platform.commons.util.ToStringBuilder; + /** * @since 1.3 */ class SingleLock implements ResourceLock { + private final List resources; private final Lock lock; - SingleLock(Lock lock) { + SingleLock(ExclusiveResource resource, Lock lock) { + this.resources = singletonList(resource); this.lock = lock; } + @Override + public List getResources() { + return resources; + } + // for tests only Lock getLock() { return this.lock; @@ -40,6 +53,18 @@ public void release() { this.lock.unlock(); } + @Override + public boolean isExclusive() { + return resources.get(0).getLockMode() == ExclusiveResource.LockMode.READ_WRITE; + } + + @Override + public String toString() { + return new ToStringBuilder(this) // + .append("resource", getOnlyElement(resources)) // + .toString(); + } + private class SingleLockManagedBlocker implements ForkJoinPool.ManagedBlocker { private volatile boolean acquired; @@ -59,21 +84,4 @@ 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/CompositeLockTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/CompositeLockTests.java index 756c710e25c6..75e0a262f98e 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/CompositeLockTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/CompositeLockTests.java @@ -19,9 +19,11 @@ import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.locks.Lock; +import java.util.stream.IntStream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode; /** * @since 1.3 @@ -34,7 +36,7 @@ void acquiresAllLocksInOrder() throws Exception { var lock1 = mock(Lock.class); var lock2 = mock(Lock.class); - new CompositeLock(List.of(lock1, lock2)).acquire(); + new CompositeLock(anyResources(2), List.of(lock1, lock2)).acquire(); var inOrder = inOrder(lock1, lock2); inOrder.verify(lock1).lockInterruptibly(); @@ -47,7 +49,7 @@ void releasesAllLocksInReverseOrder() throws Exception { var lock1 = mock(Lock.class); var lock2 = mock(Lock.class); - new CompositeLock(List.of(lock1, lock2)).acquire().close(); + new CompositeLock(anyResources(2), List.of(lock1, lock2)).acquire().close(); var inOrder = inOrder(lock1, lock2); inOrder.verify(lock2).unlock(); @@ -64,7 +66,7 @@ void releasesLocksInReverseOrderWhenInterruptedDuringAcquire() throws Exception var thread = new Thread(() -> { try { - new CompositeLock(List.of(firstLock, secondLock, unavailableLock)).acquire(); + new CompositeLock(anyResources(3), List.of(firstLock, secondLock, unavailableLock)).acquire(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); @@ -90,4 +92,10 @@ private Lock mockLock(String name, Executable lockAction) throws InterruptedExce return lock; } + private List anyResources(int n) { + return IntStream.range(0, n) // + .mapToObj(j -> new ExclusiveResource("key" + j, LockMode.READ)) // + .toList(); + } + } 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 f3ee3d923ab6..2e0648df63bf 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 @@ -10,30 +10,48 @@ package org.junit.platform.engine.support.hierarchical; +import static java.util.concurrent.TimeUnit.SECONDS; 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.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.params.provider.Arguments.arguments; 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.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; 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.Executable; import org.junit.jupiter.api.function.ThrowingConsumer; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.platform.commons.JUnitException; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode; 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; +@Timeout(5) class ForkJoinPoolHierarchicalTestExecutorServiceTests { + DummyTaskFactory taskFactory = new DummyTaskFactory(); + LockManager lockManager = new LockManager(); + @Test void exceptionsFromInvalidConfigurationAreNotSwallowed() { var configuration = new DefaultParallelExecutionConfiguration(2, 1, 1, 1, 0, __ -> true); @@ -48,17 +66,53 @@ 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); + static List incompatibleLockCombinations() { + return List.of(// + arguments(// + Set.of(GLOBAL_READ), // + Set.of(GLOBAL_READ_WRITE) // + ), // + arguments(// + Set.of(new ExclusiveResource("a", LockMode.READ)), // + Set.of(new ExclusiveResource("a", LockMode.READ_WRITE)) // + ), // + arguments(// + Set.of(new ExclusiveResource("a", LockMode.READ_WRITE)), // + Set.of(new ExclusiveResource("a", LockMode.READ_WRITE)) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ_WRITE)), // + Set.of(GLOBAL_READ, new ExclusiveResource("b", LockMode.READ_WRITE)) // + ), // + arguments(// + Set.of(new ExclusiveResource("b", LockMode.READ)), // + Set.of(new ExclusiveResource("a", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ_WRITE)), // + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ_WRITE), // + Set.of(GLOBAL_READ) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ), + new ExclusiveResource("b", LockMode.READ), new ExclusiveResource("d", LockMode.READ)), + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ), + new ExclusiveResource("c", LockMode.READ)) // + )// + ); + } + + @ParameterizedTest + @MethodSource("incompatibleLockCombinations") + void defersTasksWithIncompatibleLocks(Set initialResources, + Set incompatibleResources) throws Throwable { - var lockManager = new LockManager(); - var globalReadLock = lockManager.getLockForResource(GLOBAL_READ); - var globalReadWriteLock = lockManager.getLockForResource(GLOBAL_READ_WRITE); - var nopLock = NopLock.INSTANCE; + var initialLock = lockManager.getLockForResources(initialResources); + var incompatibleLock = lockManager.getLockForResources(incompatibleResources); - var threadNamesByTaskIdentifier = new ConcurrentHashMap(); var deferred = new CountDownLatch(1); var deferredTask = new AtomicReference(); @@ -67,44 +121,198 @@ void defersTasksWithIncompatibleLocks() throws Exception { deferred.countDown(); }; - var isolatedTask = new DummyTestTask("isolatedTask", globalReadWriteLock, - t -> threadNamesByTaskIdentifier.put(t.identifier(), Thread.currentThread().getName())); + var incompatibleTask = taskFactory.create("incompatibleTask", incompatibleLock); - try (var pool = new ForkJoinPoolHierarchicalTestExecutorService(configuration, taskEventListener)) { + var tasks = runWithAttemptedWorkStealing(taskEventListener, incompatibleTask, initialLock, + () -> await(deferred, "Interrupted while waiting for task to be deferred")); - 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(); + assertEquals(incompatibleTask, deferredTask.get()); + assertEquals(tasks.get("nestedTask").threadName, tasks.get("leafTaskB").threadName); + assertNotEquals(tasks.get("leafTaskA").threadName, tasks.get("leafTaskB").threadName); + } + + static List compatibleLockCombinations() { + return List.of(// + arguments(// + Set.of(GLOBAL_READ), // + Set.of(new ExclusiveResource("a", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ), // + Set.of(new ExclusiveResource("a", LockMode.READ_WRITE)) // + ), // + arguments(// + Set.of(GLOBAL_READ), // + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ_WRITE)) // + ), // + arguments(// + Set.of(GLOBAL_READ), // + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ)), // + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ), + new ExclusiveResource("b", LockMode.READ), new ExclusiveResource("c", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ)), // + Set.of(GLOBAL_READ, new ExclusiveResource("b", LockMode.READ)) // + ), // + arguments(// + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ)), // + Set.of(new ExclusiveResource("a", LockMode.READ), new ExclusiveResource("b", LockMode.READ), + new ExclusiveResource("c", LockMode.READ)) // + )// + ); + } + + @ParameterizedTest + @MethodSource("compatibleLockCombinations") + void canWorkStealTaskWithCompatibleLocks(Set initialResources, + Set compatibleResources) throws Throwable { + + var initialLock = lockManager.getLockForResources(initialResources); + var compatibleLock = lockManager.getLockForResources(compatibleResources); + + var deferredTask = new AtomicReference(); + + var workStolen = new CountDownLatch(1); + var compatibleTask = taskFactory.create("compatibleTask", compatibleLock, workStolen::countDown); + + var tasks = runWithAttemptedWorkStealing(deferredTask::set, compatibleTask, initialLock, + () -> await(workStolen, "Interrupted while waiting for work to be stolen")); + + assertNull(deferredTask.get()); + assertEquals(tasks.get("nestedTask").threadName, tasks.get("leafTaskB").threadName); + assertNotEquals(tasks.get("leafTaskA").threadName, tasks.get("leafTaskB").threadName); + } + + @Test + void defersTasksWithIncompatibleLocksOnMultipleLevels() throws Throwable { + + var initialLock = lockManager.getLockForResources( + Set.of(GLOBAL_READ, new ExclusiveResource("a", LockMode.READ))); + var incompatibleLock1 = lockManager.getLockForResource(new ExclusiveResource("a", LockMode.READ_WRITE)); + var compatibleLock1 = lockManager.getLockForResource(new ExclusiveResource("b", LockMode.READ)); + var incompatibleLock2 = lockManager.getLockForResource(new ExclusiveResource("b", LockMode.READ_WRITE)); + + var deferred = new ConcurrentHashMap(); + var deferredTasks = new CopyOnWriteArrayList(); + TaskEventListener taskEventListener = testTask -> { + deferredTasks.add(testTask); + deferred.get(testTask).countDown(); + }; + + var incompatibleTask1 = taskFactory.create("incompatibleTask1", incompatibleLock1); + deferred.put(incompatibleTask1, new CountDownLatch(1)); + + var incompatibleTask2 = taskFactory.create("incompatibleTask2", incompatibleLock2); + deferred.put(incompatibleTask2, new CountDownLatch(1)); + + var configuration = new DefaultParallelExecutionConfiguration(2, 2, 2, 2, 1, __1 -> true); + + withForkJoinPoolHierarchicalTestExecutorService(configuration, taskEventListener, service -> { + + var nestedTask2 = createNestedTaskWithTwoConcurrentLeafTasks(service, "2", compatibleLock1, + List.of(incompatibleTask2), // + () -> await(deferred.get(incompatibleTask2), incompatibleTask2.identifier + " to be deferred")); + + var nestedTask1 = createNestedTaskWithTwoConcurrentLeafTasks(service, "1", initialLock, + List.of(incompatibleTask1, nestedTask2), // + () -> { + await(deferred.get(incompatibleTask1), incompatibleTask1.identifier + " to be deferred"); + await(nestedTask2.started, nestedTask2.identifier + " to be started"); }); - pool.invokeAll(List.of(leafTask1, leafTask2)); + + service.submit(nestedTask1).get(); + }); + + assertThat(deferredTasks) // + .containsExactly(incompatibleTask1, incompatibleTask2, incompatibleTask1); // incompatibleTask1 may be deferred multiple times + assertThat(taskFactory.tasks) // + .hasSize(3 + 3 + 2) // + .values().extracting(it -> it.completion.isDone()).containsOnly(true); + assertThat(taskFactory.tasks) // + .values().extracting(it -> it.completion.isCompletedExceptionally()).containsOnly(false); + } + + private Map runWithAttemptedWorkStealing(TaskEventListener taskEventListener, + DummyTestTask taskToBeStolen, ResourceLock initialLock, Runnable waitAction) throws Throwable { + + var configuration = new DefaultParallelExecutionConfiguration(2, 2, 2, 2, 1, __ -> true); + + withForkJoinPoolHierarchicalTestExecutorService(configuration, taskEventListener, service -> { + + var nestedTask = createNestedTaskWithTwoConcurrentLeafTasks(service, "", initialLock, + List.of(taskToBeStolen), waitAction); + + service.submit(nestedTask).get(); + }); + + return taskFactory.tasks; + } + + private DummyTestTask createNestedTaskWithTwoConcurrentLeafTasks( + ForkJoinPoolHierarchicalTestExecutorService service, String identifierSuffix, ResourceLock parentLock, + List tasksToFork, Runnable waitAction) { + + return taskFactory.create("nestedTask" + identifierSuffix, parentLock, () -> { + + var bothLeafTasksAreRunning = new CountDownLatch(2); + + var leafTaskA = taskFactory.create("leafTaskA" + identifierSuffix, NopLock.INSTANCE, () -> { + tasksToFork.forEach(task -> service.new ExclusiveTask(task).fork()); + bothLeafTasksAreRunning.countDown(); + bothLeafTasksAreRunning.await(); + waitAction.run(); + }); + + var leafTaskB = taskFactory.create("leafTaskB" + identifierSuffix, NopLock.INSTANCE, () -> { + bothLeafTasksAreRunning.countDown(); + bothLeafTasksAreRunning.await(); }); - pool.submit(nestedTask).get(); + service.invokeAll(List.of(leafTaskA, leafTaskB)); + }); + } + + private static void await(CountDownLatch latch, String message) { + try { + latch.await(); + } + catch (InterruptedException e) { + System.out.println("Interrupted while waiting for " + message); } + } + + private void withForkJoinPoolHierarchicalTestExecutorService(ParallelExecutionConfiguration configuration, + TaskEventListener taskEventListener, ThrowingConsumer action) + throws Throwable { + try (var service = new ForkJoinPoolHierarchicalTestExecutorService(configuration, taskEventListener)) { + + action.accept(service); - assertEquals(isolatedTask, deferredTask.get()); - assertEquals(threadNamesByTaskIdentifier.get("nestedTask"), threadNamesByTaskIdentifier.get("leafTask2")); - assertNotEquals(threadNamesByTaskIdentifier.get("leafTask1"), threadNamesByTaskIdentifier.get("leafTask2")); + service.forkJoinPool.shutdown(); + assertTrue(service.forkJoinPool.awaitTermination(5, SECONDS), "Pool did not terminate within timeout"); + } } - record DummyTestTask(String identifier, ResourceLock resourceLock, ThrowingConsumer action) - implements TestTask { + static final class DummyTestTask implements TestTask { + + private final String identifier; + private final ResourceLock resourceLock; + private final Executable action; + + private volatile String threadName; + private final CountDownLatch started = new CountDownLatch(1); + private final CompletableFuture completion = new CompletableFuture<>(); + + DummyTestTask(String identifier, ResourceLock resourceLock, Executable action) { + this.identifier = identifier; + this.resourceLock = resourceLock; + this.action = action; + } + @Override public ExecutionMode getExecutionMode() { return CONCURRENT; @@ -117,10 +325,14 @@ public ResourceLock getResourceLock() { @Override public void execute() { + threadName = Thread.currentThread().getName(); + started.countDown(); try { - action.accept(this); + action.execute(); + completion.complete(null); } catch (Throwable e) { + completion.completeExceptionally(e); throw new RuntimeException("Action " + identifier + " failed", e); } } @@ -130,4 +342,20 @@ public String toString() { return identifier; } } + + static final class DummyTaskFactory { + + final Map tasks = new HashMap<>(); + + DummyTestTask create(String identifier, ResourceLock resourceLock) { + return create(identifier, resourceLock, () -> { + }); + } + + DummyTestTask create(String identifier, ResourceLock resourceLock, Executable action) { + DummyTestTask task = new DummyTestTask(identifier, resourceLock, action); + tasks.put(task.identifier, task); + return task; + } + } } 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 c9cb0570027e..a1cb3efd1483 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 @@ -112,20 +112,20 @@ void globalLockComesFirst(LockMode globalLockMode) { } @Test - void usesSpecialClassForGlobalReadLock() { + void usesSingleInstanceForGlobalReadLock() { var lock = lockManager.getLockForResources(List.of(ExclusiveResource.GLOBAL_READ)); assertThat(lock) // - .isInstanceOf(SingleLock.GlobalReadLock.class) // + .isInstanceOf(SingleLock.class) // .isSameAs(lockManager.getLockForResource(ExclusiveResource.GLOBAL_READ)); } @Test - void usesSpecialClassForGlobalReadWriteLock() { + void usesSingleInstanceForGlobalReadWriteLock() { var lock = lockManager.getLockForResources(List.of(ExclusiveResource.GLOBAL_READ_WRITE)); assertThat(lock) // - .isInstanceOf(SingleLock.GlobalReadWriteLock.class) // + .isInstanceOf(SingleLock.class) // .isSameAs(lockManager.getLockForResource(ExclusiveResource.GLOBAL_READ_WRITE)); } 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 index ca80c33d6131..2372ff27de42 100644 --- 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 @@ -12,71 +12,139 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ; +import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE; +import java.util.Arrays; import java.util.List; +import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import org.junit.jupiter.api.Test; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode; -@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())))); + void nopLocks() { + assertCompatible(nopLock(), nopLock()); + assertCompatible(nopLock(), singleLock(anyReadOnlyResource())); + assertCompatible(nopLock(), compositeLock(anyReadOnlyResource())); } @Test - void singleLocksAreIncompatibleWithNonNopLocks() { - var singleLock = new SingleLock(anyReentrantLock()); + void readOnlySingleLocks() { + ExclusiveResource bR = readOnlyResource("b"); + + assertCompatible(singleLock(bR), nopLock()); + assertCompatible(singleLock(bR), singleLock(bR)); + assertIncompatible(singleLock(bR), singleLock(readWriteResource("b")), "read-write conflict"); + assertIncompatible(singleLock(bR), singleLock(readOnlyResource("a")), "lock acquisition order"); + assertCompatible(singleLock(bR), singleLock(readOnlyResource("c"))); + assertIncompatible(singleLock(bR), singleLock(GLOBAL_READ), "lock acquisition order"); + assertIncompatible(singleLock(bR), singleLock(GLOBAL_READ_WRITE), "lock acquisition order"); + assertCompatible(singleLock(bR), compositeLock(bR, readOnlyResource("c"))); + assertIncompatible(singleLock(bR), compositeLock(readOnlyResource("a1"), readOnlyResource("a2"), bR), + "lock acquisition order"); + } - 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 readWriteSingleLocks() { + ExclusiveResource bRW = readWriteResource("b"); + + assertCompatible(singleLock(bRW), nopLock()); + assertIncompatible(singleLock(bRW), singleLock(bRW), "isolation guarantees"); + assertIncompatible(singleLock(bRW), compositeLock(bRW), "isolation guarantees"); + assertIncompatible(singleLock(bRW), singleLock(readOnlyResource("a")), "lock acquisition order"); + assertIncompatible(singleLock(bRW), singleLock(readOnlyResource("b")), "isolation guarantees"); + assertIncompatible(singleLock(bRW), singleLock(readOnlyResource("c")), "isolation guarantees"); + assertIncompatible(singleLock(bRW), singleLock(GLOBAL_READ), "lock acquisition order"); + assertIncompatible(singleLock(bRW), singleLock(GLOBAL_READ_WRITE), "lock acquisition order"); + assertIncompatible(singleLock(bRW), compositeLock(bRW, readOnlyResource("c")), "isolation guarantees"); + assertIncompatible(singleLock(bRW), compositeLock(readOnlyResource("a1"), readOnlyResource("a2"), bRW), + "lock acquisition order"); } @Test - void globalReadLockIsCompatibleWithEverythingExceptGlobalReadWriteLock() { - var globalReadLock = new SingleLock.GlobalReadLock(anyReentrantLock()); + void globalReadLock() { + assertCompatible(singleLock(GLOBAL_READ), nopLock()); + assertCompatible(singleLock(GLOBAL_READ), singleLock(GLOBAL_READ)); + assertCompatible(singleLock(GLOBAL_READ), singleLock(anyReadOnlyResource())); + assertCompatible(singleLock(GLOBAL_READ), singleLock(anyReadWriteResource())); + assertIncompatible(singleLock(GLOBAL_READ), singleLock(GLOBAL_READ_WRITE), "read-write conflict"); + } - 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 readOnlyCompositeLocks() { + ExclusiveResource bR = readOnlyResource("b"); + + assertCompatible(compositeLock(bR), nopLock()); + assertCompatible(compositeLock(bR), singleLock(bR)); + assertCompatible(compositeLock(bR), compositeLock(bR)); + assertIncompatible(compositeLock(bR), singleLock(GLOBAL_READ), "lock acquisition order"); + assertIncompatible(compositeLock(bR), singleLock(GLOBAL_READ_WRITE), "lock acquisition order"); + assertIncompatible(compositeLock(bR), compositeLock(readOnlyResource("a")), "lock acquisition order"); + assertCompatible(compositeLock(bR), compositeLock(readOnlyResource("c"))); + assertIncompatible(compositeLock(bR), compositeLock(readWriteResource("b")), "read-write conflict"); + assertIncompatible(compositeLock(bR), compositeLock(bR, readWriteResource("b")), "read-write conflict"); } @Test - void globalReadWriteLockIsIncompatibleWithWithNonNopLocks() { - var globalReadWriteLock = new SingleLock.GlobalReadWriteLock(anyReentrantLock()); + void readWriteCompositeLocks() { + ExclusiveResource bRW = readWriteResource("b"); + + assertCompatible(compositeLock(bRW), nopLock()); + assertIncompatible(compositeLock(bRW), singleLock(bRW), "isolation guarantees"); + assertIncompatible(compositeLock(bRW), compositeLock(bRW), "isolation guarantees"); + assertIncompatible(compositeLock(bRW), singleLock(readOnlyResource("a")), "lock acquisition order"); + assertIncompatible(compositeLock(bRW), singleLock(readOnlyResource("b")), "isolation guarantees"); + assertIncompatible(compositeLock(bRW), singleLock(readOnlyResource("c")), "isolation guarantees"); + assertIncompatible(compositeLock(bRW), singleLock(GLOBAL_READ), "lock acquisition order"); + assertIncompatible(compositeLock(bRW), singleLock(GLOBAL_READ_WRITE), "lock acquisition order"); + assertIncompatible(compositeLock(bRW), compositeLock(readOnlyResource("a")), "lock acquisition order"); + assertIncompatible(compositeLock(bRW), compositeLock(readOnlyResource("b"), readOnlyResource("c")), + "isolation guarantees"); + } - 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())))); + private static void assertCompatible(ResourceLock first, ResourceLock second) { + assertTrue(first.isCompatible(second), + "Expected locks to be compatible:\n(1) %s\n(2) %s".formatted(first, second)); } - @Test - void compositeLocksAreIncompatibleWithNonNopLocks() { - CompositeLock compositeLock = new CompositeLock(List.of(anyReentrantLock())); + private static void assertIncompatible(ResourceLock first, ResourceLock second, String reason) { + assertFalse(first.isCompatible(second), + "Expected locks to be incompatible due to %s:\n(1) %s\n(2) %s".formatted(reason, first, second)); + } + + private static ResourceLock nopLock() { + return NopLock.INSTANCE; + } + + private static SingleLock singleLock(ExclusiveResource resource) { + return new SingleLock(resource, anyLock()); + } + + private static CompositeLock compositeLock(ExclusiveResource... resources) { + return new CompositeLock(List.of(resources), Arrays.stream(resources).map(__ -> anyLock()).toList()); + } + + private static ExclusiveResource anyReadOnlyResource() { + return readOnlyResource("key"); + } + + private static ExclusiveResource anyReadWriteResource() { + return readWriteResource("key"); + } + + private static ExclusiveResource readOnlyResource(String key) { + return new ExclusiveResource(key, LockMode.READ); + } - 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 ExclusiveResource readWriteResource(String key) { + return new ExclusiveResource(key, LockMode.READ_WRITE); } - private static ReentrantLock anyReentrantLock() { + private static Lock anyLock() { return new ReentrantLock(); } } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/SingleLockTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/SingleLockTests.java index edf36cf66d4e..d4a9ef75e0cb 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/SingleLockTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/SingleLockTests.java @@ -16,6 +16,7 @@ import java.util.concurrent.locks.ReentrantLock; import org.junit.jupiter.api.Test; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode; /** * @since 1.3 @@ -27,7 +28,7 @@ class SingleLockTests { void acquire() throws Exception { var lock = new ReentrantLock(); - new SingleLock(lock).acquire(); + new SingleLock(anyResource(), lock).acquire(); assertTrue(lock.isLocked()); } @@ -37,9 +38,13 @@ void acquire() throws Exception { void release() throws Exception { var lock = new ReentrantLock(); - new SingleLock(lock).acquire().close(); + new SingleLock(anyResource(), lock).acquire().close(); assertFalse(lock.isLocked()); } + private static ExclusiveResource anyResource() { + return new ExclusiveResource("key", LockMode.READ); + } + }