Bump PMD

build.gradle.kts

@@ -12,6 +12,7 @@ val errorProneVersion = "2.48.0"
 val ktlintVersion = "1.8.0"
 val pitestMainVersion = "1.23.0"
 val pitestJUnit5PluginVersion = "1.2.3"
+val pmdVersion = "7.23.0"
 ext["jmhVersion"] = "1.37"
 
 configurations {
@@ -75,6 +76,7 @@ allprojects {
 
                 apply(plugin = "pmd")
                 configure<PmdExtension> {
+                    toolVersion = pmdVersion
                     ruleSets = listOf()
                     ruleSetConfig = resources.text.fromFile(rootProject.file("config/pmd/rulesets.xml"))
                 }

config/pmd/rulesets.xml

@@ -7,24 +7,21 @@
 
   <rule ref="category/java/bestpractices.xml">
     <exclude name="AvoidReassigningParameters"/>
-    <exclude name="JUnitAssertionsShouldIncludeMessage"/>
-    <exclude name="JUnitTestContainsTooManyAsserts"/>
-    <exclude name="JUnitTestsShouldIncludeAssert"/>
+    <exclude name="UnitTestAssertionsShouldIncludeMessage"/>
+    <exclude name="UnitTestContainsTooManyAsserts"/>
+    <exclude name="UnitTestShouldIncludeAssert"/>
     <!-- It is better to crash if a null is unexpected. -->
     <exclude name="LiteralsFirstInComparisons"/>
   </rule>
 
   <rule ref="category/java/codestyle.xml">
-    <exclude name="AbstractNaming"/>
     <exclude name="AtLeastOneConstructor"/>
-    <exclude name="AvoidFinalLocalVariable"/>
     <exclude name="AvoidUsingNativeCode"/>
     <exclude name="BooleanGetMethodName"/>
     <exclude name="CallSuperInConstructor"/>
     <exclude name="ClassNamingConventions"/>
     <exclude name="CommentDefaultAccessModifier"/>
     <exclude name="ConfusingTernary"/>
-    <exclude name="DefaultPackage"/>
     <exclude name="FieldDeclarationsShouldBeAtStartOfClass"/>
     <exclude name="GenericsNaming"/>
     <exclude name="LinguisticNaming"/>
@@ -36,7 +33,6 @@
     <exclude name="ShortMethodName"/>
     <exclude name="ShortVariable"/>
     <exclude name="TooManyStaticImports"/>
-    <exclude name="VariableNamingConventions"/>
     <exclude name="UnnecessaryFullyQualifiedName"/>
   </rule>
 
@@ -49,23 +45,16 @@
     <exclude name="CouplingBetweenObjects"/>
     <exclude name="CyclomaticComplexity"/>
     <exclude name="DataClass"/>
-    <exclude name="ExcessiveClassLength"/>
     <exclude name="ExcessiveImports"/>
-    <exclude name="ExcessiveMethodLength"/>
     <exclude name="ExcessiveParameterList"/>
     <exclude name="ExcessivePublicCount"/>
     <exclude name="GodClass"/>
     <exclude name="LawOfDemeter"/>
     <exclude name="LoosePackageCoupling"/>
-    <exclude name="ModifiedCyclomaticComplexity"/>
     <exclude name="NPathComplexity"/>
-    <exclude name="NcssConstructorCount"/>
     <exclude name="NcssCount"/>
-    <exclude name="NcssMethodCount"/>
-    <exclude name="NcssTypeCount"/>
     <exclude name="SignatureDeclareThrowsException"/>
     <exclude name="SimplifiedTernary"/>
-    <exclude name="StdCyclomaticComplexity"/>
     <exclude name="SwitchDensity"/>
     <exclude name="TooManyFields"/>
     <exclude name="TooManyMethods"/>
@@ -86,7 +75,6 @@
     <exclude name="CallSuperFirst"/>
     <exclude name="CallSuperLast"/>
     <exclude name="CloseResource"/>
-    <exclude name="DataflowAnomalyAnalysis"/>
     <exclude name="DoNotTerminateVM"/>
     <exclude name="NonSerializableClass"/>
     <exclude name="NullAssignment"/>
@@ -95,6 +83,7 @@
   </rule>
 
   <rule ref="category/java/multithreading.xml">
+    <exclude name="AvoidThreadGroup"/>
     <exclude name="DoNotUseThreads"/>
     <exclude name="UseConcurrentHashMap"/>
   </rule>

driver/src/main/java/io/github/davidburstrom/contester/ConTesterDriver.java

@@ -29,6 +29,8 @@
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
 
@@ -59,14 +61,14 @@
  *       using {@link #join} and {@link #getUncaughtThrowable}.
  * </ol>
  */
-@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
 public final class ConTesterDriver {
 
   /** Standard timeout, in milliseconds, for blocking APIs. */
   public static final long STANDARD_TIMEOUT_MS = 10_000;
 
   private static final Map<Thread, DriverData> DRIVER_REGISTRY =
       Collections.synchronizedMap(new WeakHashMap<>());
+  private static final ReentrantLock DRIVER_REGISTRY_LOCK = new ReentrantLock();
 
   /** Prohibit instantiation */
   private ConTesterDriver() {}
@@ -271,11 +273,14 @@ public static void waitForBreakpoint(Thread thread, String id, long timeout, Tim
 
     final long endTime = System.nanoTime() + timeUnit.toNanos(timeout);
     final ThreadData threadData = driverData.getThreadRegistry().get(thread);
-    synchronized (threadData) {
+    threadData.lock.lock();
+    try {
+      Condition condition = threadData.lock.newCondition();
       while (threadData.getSuspended() == null && System.nanoTime() < endTime) {
 
         try {
-          threadData.wait(1);
+          //noinspection ResultOfMethodCallIgnored
+          condition.awaitNanos(1);
         } catch (InterruptedException e) {
           throw new RuntimeException(e);
         }
@@ -300,6 +305,8 @@ public static void waitForBreakpoint(Thread thread, String id, long timeout, Tim
         throw new AssertionError(
             "Thread suspended on unexpected breakpoint '" + threadData.getSuspended());
       }
+    } finally {
+      threadData.lock.unlock();
     }
   }
 
@@ -391,13 +398,16 @@ public static void waitForBlockedOrTerminated(Thread thread, long timeout, TimeU
   public static void resume(Thread thread) {
     final DriverData driverData = DRIVER_REGISTRY.get(Thread.currentThread());
     final ThreadData threadData = driverData.getThreadRegistry().get(thread);
-    synchronized (threadData) {
+    threadData.lock.lock();
+    try {
       if (threadData.getSuspended() != null) {
         threadData.setSuspended(null);
         threadData.semaphore.release();
       } else {
         throw new AssertionError("Thread is not suspended");
       }
+    } finally {
+      threadData.lock.unlock();
     }
   }
 
@@ -428,8 +438,11 @@ public static void join(final Thread thread, long timeout, TimeUnit timeUnit) {
 
     final DriverData driverData = DRIVER_REGISTRY.get(Thread.currentThread());
     final Map<String, Set<Thread>> enabledBreakpoints = driverData.getEnabledBreakpoints();
-    synchronized (enabledBreakpoints) {
+    driverData.lock.lock();
+    try {
       enabledBreakpoints.forEach((id, threads) -> threads.remove(thread));
+    } finally {
+      driverData.lock.unlock();
     }
 
     if (isSuspended(thread)) {
@@ -517,18 +530,23 @@ static void visitBreakpoint(String id, BooleanSupplier condition) {
     final Map<String, Set<Thread>> enabledBreakpoints = driverData.get().getEnabledBreakpoints();
     final ThreadData threadData = driverData.get().getThreadRegistry().get(Thread.currentThread());
     final boolean suspend;
-    synchronized (enabledBreakpoints) {
+    driverData.get().lock.lock();
+    try {
       final Set<Thread> set = enabledBreakpoints.get(id);
       if (set != null && set.contains(Thread.currentThread()) && condition.getAsBoolean()) {
-        synchronized (threadData) {
+        threadData.lock.lock();
+        try {
           suspend = true;
           threadData.setSuspended(id);
           // This can only be mutation tested by injecting a custom wait time in waitForBreakpoint
-          threadData.notifyAll();
+        } finally {
+          threadData.lock.unlock();
         }
       } else {
         suspend = false;
       }
+    } finally {
+      driverData.get().lock.unlock();
     }
     if (suspend) {
       try {
@@ -540,8 +558,11 @@ static void visitBreakpoint(String id, BooleanSupplier condition) {
   }
 
   private static DriverData getOrCreateDriverData() {
-    synchronized (DRIVER_REGISTRY) {
+    DRIVER_REGISTRY_LOCK.lock();
+    try {
       return DRIVER_REGISTRY.computeIfAbsent(Thread.currentThread(), t -> new DriverData());
+    } finally {
+      DRIVER_REGISTRY_LOCK.unlock();
     }
   }
 
@@ -550,12 +571,14 @@ private static Optional<ThreadData> findThreadData(Thread thread) {
     if (DRIVER_REGISTRY.isEmpty()) {
       return Optional.empty();
     }
-
-    synchronized (DRIVER_REGISTRY) {
+    DRIVER_REGISTRY_LOCK.lock();
+    try {
       return DRIVER_REGISTRY.values().stream()
           .map(driverData -> driverData.getThreadRegistry().get(thread))
           .filter(Objects::nonNull)
           .findFirst();
+    } finally {
+      DRIVER_REGISTRY_LOCK.unlock();
     }
   }
 
@@ -564,12 +587,15 @@ private static Optional<DriverData> findDriverData(Thread thread) {
     if (DRIVER_REGISTRY.isEmpty()) {
       return Optional.empty();
     }
+    DRIVER_REGISTRY_LOCK.lock();
+    try {
 
-    synchronized (DRIVER_REGISTRY) {
       return DRIVER_REGISTRY.entrySet().stream()
           .filter(entry -> entry.getValue().getThreadRegistry().containsKey(thread))
           .findFirst()
           .map(Map.Entry::getValue);
+    } finally {
+      DRIVER_REGISTRY_LOCK.unlock();
     }
   }
 
@@ -602,8 +628,11 @@ private static void startIfNecessary(final Thread thread) {
   private static boolean isSuspended(Thread thread) {
     final DriverData driverData = DRIVER_REGISTRY.get(Thread.currentThread());
     final ThreadData threadData = driverData.getThreadRegistry().get(thread);
-    synchronized (threadData) {
+    threadData.lock.lock();
+    try {
       return threadData.getSuspended() != null;
+    } finally {
+      threadData.lock.unlock();
     }
   }
 
@@ -622,11 +651,12 @@ private static Set<String> getEnabledBreakpoints(final Thread thread) {
   }
 
   /** Holds the data associated with a given driver thread, e.g. a test worker thread. */
-  private static class DriverData {
+  private static final class DriverData {
 
     private final Map<Thread, ThreadData> threadRegistry = new WeakHashMap<>();
     private final Map<String, Set<Thread>> enabledBreakpoints = new HashMap<>(4);
     private final AtomicInteger threadIdGenerator = new AtomicInteger(1);
+    private final ReentrantLock lock = new ReentrantLock();
 
     private int getNextThreadId() {
       return threadIdGenerator.getAndIncrement();
@@ -641,11 +671,12 @@ private Map<String, Set<Thread>> getEnabledBreakpoints() {
     }
   }
 
-  private static class ThreadData {
+  private static final class ThreadData {
 
     private Throwable uncaughtThrowable;
     private String breakpointId;
     private final Semaphore semaphore = new Semaphore(0);
+    private final ReentrantLock lock = new ReentrantLock();
 
     Optional<Throwable> getUncaughtThrowable() {
       return Optional.ofNullable(uncaughtThrowable);

driver/src/test/java/io/github/davidburstrom/contester/ConTesterDriverTest.java

@@ -39,6 +39,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
@@ -385,6 +386,7 @@ void threadIsAutomaticallyRegistered() {
     assertDoesNotThrow(() -> join(thread));
   }
 
+  @SuppressWarnings("PMD.AvoidSynchronizedStatement")
   @Test
   void runUntilBlocked() {
     Object lock = new Object();
@@ -409,6 +411,36 @@ void runUntilBlocked() {
     join(thread2);
   }
 
+  @Test
+  void runUntilWaiting() {
+    ReentrantLock lock = new ReentrantLock();
+    final Thread thread1 =
+        thread(
+            () -> {
+              lock.lock();
+              try {
+                visitBreakpoint("id");
+              } finally {
+                lock.unlock();
+              }
+            });
+    final Thread thread2 =
+        thread(
+            () -> {
+              lock.lock();
+              try {
+                visitBreakpoint("dummy");
+              } finally {
+                lock.unlock();
+              }
+            });
+    runToBreakpoint(thread1, "id");
+    runUntilBlockedOrTerminated(thread2);
+    assertEquals(Thread.State.WAITING, thread2.getState());
+    join(thread1);
+    join(thread2);
+  }
+
   @Test
   void runUntilBlockedOrTerminatedResumesAutomatically() {
     final Thread thread = thread(() -> visitBreakpoint("id"));

examples/src/main/java/io/github/davidburstrom/contester/examples/Modification.java

@@ -16,6 +16,7 @@
 package io.github.davidburstrom.contester.examples;
 
 import io.github.davidburstrom.contester.ConTesterBreakpoint;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Simulates an issue where one method is modifying a field while another method is referencing it,
@@ -49,25 +50,31 @@ public void print() {
   }
 
   class Fixed implements Modification {
-    final Object lock = new Object();
+    final ReentrantLock lock = new ReentrantLock();
     private Object member = new Object();
 
     @Override
     public void reset() {
       ConTesterBreakpoint.defineBreakpoint("reset");
-      synchronized (lock) {
+      lock.lock();
+      try {
         member = null;
+      } finally {
+        lock.unlock();
       }
     }
 
     @Override
     public void print() {
       /* this synchronized block is introduced during a bug fix, and is proven to work */
-      synchronized (lock) {
+      lock.lock();
+      try {
         if (member != null) {
           ConTesterBreakpoint.defineBreakpoint("print");
           System.out.println(member.getClass());
         }
+      } finally {
+        lock.unlock();
       }
     }
   }