Skip to content

Commit

Permalink
Remove special classes for global locks
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcphilipp
marcphilipp committed Sep 19, 2024
1 parent 8109abb commit 9803fce
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 55 deletions.
8 changes: 6 additions & 2 deletions ...nit/platform/engine/support/hierarchical/ForkJoinPoolHierarchicalTestExecutorService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -170,7 +170,7 @@ private void forkConcurrentTasks(List<? extends TestTask> tasks, Deque<Exclusive
Deque<ExclusiveTask> sameThreadTasks, Deque<ExclusiveTask> 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) {
Expand All @@ -183,6 +183,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<ExclusiveTask> tasks) {
for (ExclusiveTask task : tasks) {
task.execSync();
Expand Down
11 changes: 4 additions & 7 deletions ...form-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,18 @@
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 final Map<String, ReadWriteLock> 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<ExclusiveResource> resources) {
Expand Down
12 changes: 0 additions & 12 deletions ...tform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleLock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,4 @@ public boolean isReleasable() {
}

}

static class GlobalReadLock extends SingleLock {
GlobalReadLock(Lock lock) {
super(ExclusiveResource.GLOBAL_READ, lock);
}
}

static class GlobalReadWriteLock extends SingleLock {
GlobalReadWriteLock(Lock lock) {
super(ExclusiveResource.GLOBAL_READ_WRITE, lock);
}
}
}
8 changes: 4 additions & 4 deletions ...-tests/src/test/java/org/junit/platform/engine/support/hierarchical/LockManagerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
36 changes: 6 additions & 30 deletions ...tests/src/test/java/org/junit/platform/engine/support/hierarchical/ResourceLockTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

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 static org.junit.platform.engine.support.hierarchical.ExclusiveResource.singleton;

import java.util.List;
Expand Down Expand Up @@ -46,49 +48,23 @@ void singleLocksAreIncompatibleWithOtherLocksThatCanPotentiallyCauseDeadlocks()
assertFalse(singleLock.isCompatible(new SingleLock(resource("B", LockMode.READ_WRITE), anyReentrantLock())));
assertFalse(singleLock.isCompatible(new SingleLock(resource("A"), anyReentrantLock())));
assertTrue(singleLock.isCompatible(new SingleLock(resource("C"), anyReentrantLock())));
assertFalse(singleLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock())));
assertFalse(singleLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock())));
assertFalse(singleLock.isCompatible(new SingleLock(GLOBAL_READ, anyReentrantLock())));
assertFalse(singleLock.isCompatible(new SingleLock(GLOBAL_READ_WRITE, anyReentrantLock())));
assertTrue(singleLock.isCompatible(
new CompositeLock(allOf(resourceB, resource("C")), List.of(anyReentrantLock(), anyReentrantLock()))));
assertFalse(singleLock.isCompatible(
new CompositeLock(allOf(resourceB, resource("A")), List.of(anyReentrantLock(), anyReentrantLock()))));
}

@Test
void globalReadLockIsCompatibleWithEverythingExceptGlobalReadWriteLock() {
var globalReadLock = new SingleLock.GlobalReadLock(anyReentrantLock());

assertTrue(globalReadLock.isCompatible(globalReadLock));
assertTrue(globalReadLock.isCompatible(NopLock.INSTANCE));
assertTrue(globalReadLock.isCompatible(new SingleLock(anyResource(), anyReentrantLock())));
assertTrue(globalReadLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock())));
assertFalse(globalReadLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock())));
assertTrue(
globalReadLock.isCompatible(new CompositeLock(singleton(anyResource()), List.of(anyReentrantLock()))));
}

@Test
void globalReadWriteLockIsIncompatibleOtherLocksThatCanPotentiallyCauseDeadlocks() {
var globalReadWriteLock = new SingleLock.GlobalReadWriteLock(anyReentrantLock());

assertTrue(globalReadWriteLock.isCompatible(globalReadWriteLock));
assertTrue(globalReadWriteLock.isCompatible(NopLock.INSTANCE));
assertTrue(globalReadWriteLock.isCompatible(new SingleLock(anyResource(), anyReentrantLock())));
assertTrue(globalReadWriteLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock())));
assertTrue(globalReadWriteLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock())));
assertTrue(
globalReadWriteLock.isCompatible(new CompositeLock(singleton(anyResource()), List.of(anyReentrantLock()))));
}

@Test
void compositeLocksAreIncompatibleWithOtherLocksThatCanPotentiallyCauseDeadlocks() {
CompositeLock compositeLock = new CompositeLock(singleton(anyResource()), List.of(anyReentrantLock()));

assertTrue(compositeLock.isCompatible(compositeLock));
assertTrue(compositeLock.isCompatible(NopLock.INSTANCE));
assertTrue(compositeLock.isCompatible(new SingleLock(anyResource(), anyReentrantLock())));
assertFalse(compositeLock.isCompatible(new SingleLock.GlobalReadLock(anyReentrantLock())));
assertFalse(compositeLock.isCompatible(new SingleLock.GlobalReadWriteLock(anyReentrantLock())));
assertFalse(compositeLock.isCompatible(new SingleLock(GLOBAL_READ, anyReentrantLock())));
assertFalse(compositeLock.isCompatible(new SingleLock(GLOBAL_READ_WRITE, anyReentrantLock())));
assertTrue(compositeLock.isCompatible(compositeLock));
}

Expand Down

0 comments on commit 9803fce

Please sign in to comment.