8229442: AQS and lock classes refresh

Reviewed-by: martin

src/java.base/share/classes/java/util/concurrent/locks/Lock.java

@@ -122,9 +122,8 @@ import java.util.concurrent.TimeUnit;
  * <p>All {@code Lock} implementations <em>must</em> enforce the same
  * memory synchronization semantics as provided by the built-in monitor
  * lock, as described in
- * <a href="https://docs.oracle.com/javase/specs/jls/se11/html/jls-17.html#jls-17.4">
  * Chapter 17 of
- * <cite>The Java&trade; Language Specification</cite></a>:
+ * <cite>The Java&trade; Language Specification</cite>:
  * <ul>
  * <li>A successful {@code lock} operation has the same memory
  * synchronization effects as a successful <em>Lock</em> action.
@@ -162,6 +161,7 @@ import java.util.concurrent.TimeUnit;
  * @see ReentrantLock
  * @see Condition
  * @see ReadWriteLock
+ * @jls 17.4 Memory Model
  *
  * @since 1.5
  * @author Doug Lea

src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java

@@ -140,8 +140,25 @@ public class LockSupport {
     private LockSupport() {} // Cannot be instantiated.
 
     private static void setBlocker(Thread t, Object arg) {
-        // Even though volatile, hotspot doesn't need a write barrier here.
-        U.putReference(t, PARKBLOCKER, arg);
+        U.putReferenceOpaque(t, PARKBLOCKER, arg);
+    }
+
+    /**
+     * Sets the object to be returned by invocations of {@link
+     * #getBlocker getBlocker} for the current thread. This method may
+     * be used before invoking the no-argument version of {@link
+     * LockSupport#park() park()} from non-public objects, allowing
+     * more helpful diagnostics, or retaining compatibility with
+     * previous implementations of blocking methods.  Previous values
+     * of the blocker are not automatically restored after blocking.
+     * To obtain the effects of {@code park(b}}, use {@code
+     * setCurrentBlocker(b); park(); setCurrentBlocker(null);}
+     *
+     * @param blocker the blocker object
+     * @since 14
+     */
+    public static void setCurrentBlocker(Object blocker) {
+        U.putReferenceOpaque(Thread.currentThread(), PARKBLOCKER, blocker);
     }
 
     /**
@@ -292,7 +309,7 @@ public class LockSupport {
     public static Object getBlocker(Thread t) {
         if (t == null)
             throw new NullPointerException();
-        return U.getReferenceVolatile(t, PARKBLOCKER);
+        return U.getReferenceOpaque(t, PARKBLOCKER);
     }
 
     /**
@@ -393,24 +410,6 @@ public class LockSupport {
         U.park(true, deadline);
     }
 
-    /**
-     * Returns the pseudo-randomly initialized or updated secondary seed.
-     * Copied from ThreadLocalRandom due to package access restrictions.
-     */
-    static final int nextSecondarySeed() {
-        int r;
-        Thread t = Thread.currentThread();
-        if ((r = U.getInt(t, SECONDARY)) != 0) {
-            r ^= r << 13;   // xorshift
-            r ^= r >>> 17;
-            r ^= r << 5;
-        }
-        else if ((r = java.util.concurrent.ThreadLocalRandom.current().nextInt()) == 0)
-            r = 1; // avoid zero
-        U.putInt(t, SECONDARY, r);
-        return r;
-    }
-
     /**
      * Returns the thread id for the given thread.  We must access
      * this directly rather than via method Thread.getId() because
@@ -423,11 +422,9 @@ public class LockSupport {
 
     // Hotspot implementation via intrinsics API
     private static final Unsafe U = Unsafe.getUnsafe();
-    private static final long PARKBLOCKER = U.objectFieldOffset
-            (Thread.class, "parkBlocker");
-    private static final long SECONDARY = U.objectFieldOffset
-            (Thread.class, "threadLocalRandomSecondarySeed");
-    private static final long TID = U.objectFieldOffset
-            (Thread.class, "tid");
+    private static final long PARKBLOCKER
+        = U.objectFieldOffset(Thread.class, "parkBlocker");
+    private static final long TID
+        = U.objectFieldOffset(Thread.class, "tid");
 
 }

src/java.base/share/classes/java/util/concurrent/locks/ReentrantLock.java

@@ -119,39 +119,63 @@ public class ReentrantLock implements Lock, java.io.Serializable {
         private static final long serialVersionUID = -5179523762034025860L;
 
         /**
-         * Performs non-fair tryLock.  tryAcquire is implemented in
-         * subclasses, but both need nonfair try for trylock method.
+         * Performs non-fair tryLock.
          */
         @ReservedStackAccess
-        final boolean nonfairTryAcquire(int acquires) {
-            final Thread current = Thread.currentThread();
+        final boolean tryLock() {
+            Thread current = Thread.currentThread();
             int c = getState();
             if (c == 0) {
-                if (compareAndSetState(0, acquires)) {
+                if (compareAndSetState(0, 1)) {
                     setExclusiveOwnerThread(current);
                     return true;
                 }
-            }
-            else if (current == getExclusiveOwnerThread()) {
-                int nextc = c + acquires;
-                if (nextc < 0) // overflow
+            } else if (getExclusiveOwnerThread() == current) {
+                if (++c < 0) // overflow
                     throw new Error("Maximum lock count exceeded");
-                setState(nextc);
+                setState(c);
                 return true;
             }
             return false;
         }
 
+        /**
+         * Checks for reentrancy and acquires if lock immediately
+         * available under fair vs nonfair rules. Locking methods
+         * perform initialTryLock check before relaying to
+         * corresponding AQS acquire methods.
+         */
+        abstract boolean initialTryLock();
+
+        @ReservedStackAccess
+        final void lock() {
+            if (!initialTryLock())
+                acquire(1);
+        }
+
+        @ReservedStackAccess
+        final void lockInterruptibly() throws InterruptedException {
+            if (Thread.interrupted())
+                throw new InterruptedException();
+            if (!initialTryLock())
+                acquireInterruptibly(1);
+        }
+
+        @ReservedStackAccess
+        final boolean tryLockNanos(long nanos) throws InterruptedException {
+            if (Thread.interrupted())
+                throw new InterruptedException();
+            return initialTryLock() || tryAcquireNanos(1, nanos);
+        }
+
         @ReservedStackAccess
         protected final boolean tryRelease(int releases) {
             int c = getState() - releases;
-            if (Thread.currentThread() != getExclusiveOwnerThread())
+            if (getExclusiveOwnerThread() != Thread.currentThread())
                 throw new IllegalMonitorStateException();
-            boolean free = false;
-            if (c == 0) {
-                free = true;
+            boolean free = (c == 0);
+            if (free)
                 setExclusiveOwnerThread(null);
-            }
             setState(c);
             return free;
         }
@@ -195,8 +219,31 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      */
     static final class NonfairSync extends Sync {
         private static final long serialVersionUID = 7316153563782823691L;
+
+        final boolean initialTryLock() {
+            Thread current = Thread.currentThread();
+            if (compareAndSetState(0, 1)) { // first attempt is unguarded
+                setExclusiveOwnerThread(current);
+                return true;
+            } else if (getExclusiveOwnerThread() == current) {
+                int c = getState() + 1;
+                if (c < 0) // overflow
+                    throw new Error("Maximum lock count exceeded");
+                setState(c);
+                return true;
+            } else
+                return false;
+        }
+
+        /**
+         * Acquire for non-reentrant cases after initialTryLock prescreen
+         */
         protected final boolean tryAcquire(int acquires) {
-            return nonfairTryAcquire(acquires);
+            if (getState() == 0 && compareAndSetState(0, acquires)) {
+                setExclusiveOwnerThread(Thread.currentThread());
+                return true;
+            }
+            return false;
         }
     }
 
@@ -205,26 +252,34 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      */
     static final class FairSync extends Sync {
         private static final long serialVersionUID = -3000897897090466540L;
+
         /**
-         * Fair version of tryAcquire.  Don't grant access unless
-         * recursive call or no waiters or is first.
+         * Acquires only if reentrant or queue is empty.
          */
-        @ReservedStackAccess
-        protected final boolean tryAcquire(int acquires) {
-            final Thread current = Thread.currentThread();
+        final boolean initialTryLock() {
+            Thread current = Thread.currentThread();
             int c = getState();
             if (c == 0) {
-                if (!hasQueuedPredecessors() &&
-                    compareAndSetState(0, acquires)) {
+                if (!hasQueuedThreads() && compareAndSetState(0, 1)) {
                     setExclusiveOwnerThread(current);
                     return true;
                 }
-            }
-            else if (current == getExclusiveOwnerThread()) {
-                int nextc = c + acquires;
-                if (nextc < 0)
+            } else if (getExclusiveOwnerThread() == current) {
+                if (++c < 0) // overflow
                     throw new Error("Maximum lock count exceeded");
-                setState(nextc);
+                setState(c);
+                return true;
+            }
+            return false;
+        }
+
+        /**
+         * Acquires only if thread is first waiter or empty
+         */
+        protected final boolean tryAcquire(int acquires) {
+            if (getState() == 0 && !hasQueuedPredecessors() &&
+                compareAndSetState(0, acquires)) {
+                setExclusiveOwnerThread(Thread.currentThread());
                 return true;
             }
             return false;
@@ -264,7 +319,7 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      * at which time the lock hold count is set to one.
      */
     public void lock() {
-        sync.acquire(1);
+        sync.lock();
     }
 
     /**
@@ -314,7 +369,7 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      * @throws InterruptedException if the current thread is interrupted
      */
     public void lockInterruptibly() throws InterruptedException {
-        sync.acquireInterruptibly(1);
+        sync.lockInterruptibly();
     }
 
     /**
@@ -344,7 +399,7 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      *         thread; and {@code false} otherwise
      */
     public boolean tryLock() {
-        return sync.nonfairTryAcquire(1);
+        return sync.tryLock();
     }
 
     /**
@@ -421,7 +476,7 @@ public class ReentrantLock implements Lock, java.io.Serializable {
      */
     public boolean tryLock(long timeout, TimeUnit unit)
             throws InterruptedException {
-        return sync.tryAcquireNanos(1, unit.toNanos(timeout));
+        return sync.tryLockNanos(unit.toNanos(timeout));
     }
 
     /**

test/jdk/java/util/concurrent/locks/Lock/CheckedLockLoops.java

@@ -136,7 +136,7 @@ public final class CheckedLockLoops {
             long time = timer.getTime();
             long tpi = time / (iters * nthreads);
             System.out.print("\t" + LoopHelpers.rightJustify(tpi) + " ns per update");
-            //                double secs = (double)(time) / 1000000000.0;
+            //                double secs = (double)time / 1000000000.0;
             //                System.out.print("\t " + secs + "s run time");
             System.out.println();
 

test/jdk/java/util/concurrent/locks/Lock/FlakyMutex.java

@@ -49,31 +49,51 @@ public class FlakyMutex implements Lock {
     static class MyRuntimeException extends RuntimeException {}
 
     static void checkThrowable(Throwable t) {
-        check((t instanceof MyError) ||
+        if (!((t instanceof MyError) ||
               (t instanceof MyException) ||
-              (t instanceof MyRuntimeException));
+              (t instanceof MyRuntimeException)))
+            unexpected(t);
     }
 
     static void realMain(String[] args) throws Throwable {
-        final int nThreads = 3;
+        final ThreadLocalRandom rndMain = ThreadLocalRandom.current();
+        final int nCpus = Runtime.getRuntime().availableProcessors();
+        final int maxThreads = Math.min(4, nCpus);
+        final int nThreads = rndMain.nextInt(1, maxThreads + 1);
         final int iterations = 10_000;
         final CyclicBarrier startingGate = new CyclicBarrier(nThreads);
-        final FlakyMutex mutex = new FlakyMutex();
         final ExecutorService es = Executors.newFixedThreadPool(nThreads);
+        final FlakyMutex mutex = new FlakyMutex();
         final Runnable task = () -> {
             try {
+                ThreadLocalRandom rnd = ThreadLocalRandom.current();
                 startingGate.await();
                 for (int i = 0; i < iterations; i++) {
                     for (;;) {
-                        try { mutex.lock(); break; }
-                        catch (Throwable t) { checkThrowable(t); }
+                        try {
+                            if (rnd.nextBoolean())
+                                mutex.lock();
+                            else
+                                mutex.lockInterruptibly();
+                            break;
+                        } catch (Throwable t) { checkThrowable(t); }
                     }
 
-                    try { check(! mutex.tryLock()); }
-                    catch (Throwable t) { checkThrowable(t); }
+                    if (rnd.nextBoolean()) {
+                        try {
+                            check(! mutex.tryLock());
+                        } catch (Throwable t) { checkThrowable(t); }
+                    }
 
-                    try { check(! mutex.tryLock(1, TimeUnit.MICROSECONDS)); }
-                    catch (Throwable t) { checkThrowable(t); }
+                    if (rnd.nextInt(10) == 0) {
+                        try {
+                            check(! mutex.tryLock(1, TimeUnit.MICROSECONDS));
+                        } catch (Throwable t) { checkThrowable(t); }
+                    }
+
+                    if (rnd.nextBoolean()) {
+                        check(mutex.isLocked());
+                    }
 
                     mutex.unlock();
                 }
@@ -146,7 +166,11 @@ public class FlakyMutex implements Lock {
         if (x == null ? y == null : x.equals(y)) pass();
         else fail(x + " not equal to " + y);}
     public static void main(String[] args) throws Throwable {
-        try {realMain(args);} catch (Throwable t) {unexpected(t);}
+        int runsPerTest = Integer.getInteger("jsr166.runsPerTest", 1);
+        try {
+            for (int i = runsPerTest; i--> 0; )
+                realMain(args);
+        } catch (Throwable t) { unexpected(t); }
         System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
         if (failed > 0) throw new AssertionError("Some tests failed");}
     @SuppressWarnings("unchecked")

test/jdk/java/util/concurrent/locks/Lock/TimedAcquireLeak.java

@@ -42,6 +42,8 @@ import java.io.PrintStream;
 import java.io.Reader;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
@@ -72,9 +74,9 @@ public class TimedAcquireLeak {
         return new File(bin, programName).getPath();
     }
 
-    static final String java = javaProgramPath("java");
-    static final String jmap = javaProgramPath("jmap");
-    static final String jps  = javaProgramPath("jps");
+    static final String javaPath = javaProgramPath("java");
+    static final String jmapPath = javaProgramPath("jmap");
+    static final String jpsPath  = javaProgramPath("jps");
 
     static String outputOf(Reader r) throws IOException {
         final StringBuilder sb = new StringBuilder();
@@ -159,7 +161,11 @@ public class TimedAcquireLeak {
 
     static String match(String s, String regex, int group) {
         Matcher matcher = Pattern.compile(regex).matcher(s);
-        matcher.find();
+        if (! matcher.find()) {
+            String msg = String.format(
+                "match failed: s=%s regex=%s", s, regex);
+            throw new AssertionError(msg);
+        }
         return matcher.group(group);
     }
 
@@ -171,21 +177,20 @@ public class TimedAcquireLeak {
 
     static int objectsInUse(final Process child,
                             final String childPid,
-                            final String className) {
-        final String regex =
-            "(?m)^ *[0-9]+: +([0-9]+) +[0-9]+ +\\Q"+className+"\\E(?:$| )";
-        final Callable<Integer> objectsInUse =
-            new Callable<Integer>() { public Integer call() {
-                Integer i = Integer.parseInt(
-                    match(commandOutputOf(jmap, "-histo:live", childPid),
-                          regex, 1));
-                if (i > 100)
-                    System.out.print(
-                        commandOutputOf(jmap,
-                                        "-dump:file=dump,format=b",
-                                        childPid));
-                return i;
-            }};
+                            final String classNameRegex) {
+        String regex =
+            "(?m)^ *[0-9]+: +([0-9]+) +[0-9]+ +"+classNameRegex+"(?:$| )";
+        Callable<Integer> objectsInUse = () -> {
+            int i = Integer.parseInt(
+                match(commandOutputOf(jmapPath, "-histo:live", childPid),
+                      regex, 1));
+            if (i > 100)
+                System.out.print(
+                    commandOutputOf(jmapPath,
+                                    "-dump:file=dump,format=b",
+                                    childPid));
+            return i;
+        };
         try { return rendezvousParent(child, objectsInUse); }
         catch (Throwable t) { unexpected(t); return -1; }
     }
@@ -196,26 +201,27 @@ public class TimedAcquireLeak {
             return;
 
         final String childClassName = Job.class.getName();
-        final String classToCheckForLeaks = Job.classToCheckForLeaks();
-        final String uniqueID =
-            String.valueOf(ThreadLocalRandom.current().nextInt(Integer.MAX_VALUE));
+        final String classNameRegex = Job.classNameRegexToCheckForLeaks();
+        final String uniqueID = String.valueOf(
+            ThreadLocalRandom.current().nextInt(Integer.MAX_VALUE));
 
-        final String[] jobCmd = {
-            java, "-Xmx8m", "-XX:+UsePerfData",
-            "-classpath", System.getProperty("test.class.path"),
-            childClassName, uniqueID
-        };
+        final ArrayList<String> jobCmd = new ArrayList<>();
+        Collections.addAll(
+            jobCmd, javaPath, "-Xmx8m", "-XX:+UsePerfData",
+            "-classpath", System.getProperty("test.class.path"));
+        Collections.addAll(jobCmd, Utils.getTestJavaOpts());
+        Collections.addAll(jobCmd, childClassName, uniqueID);
         final Process p = new ProcessBuilder(jobCmd).start();
         // Ensure subprocess jvm has started, so that jps can find it
         p.getInputStream().read();
         sendByte(p.getOutputStream());
 
         final String childPid =
-            match(commandOutputOf(jps, "-m"),
+            match(commandOutputOf(jpsPath, "-m"),
                   "(?m)^ *([0-9]+) +\\Q"+childClassName+"\\E *"+uniqueID+"$", 1);
 
-        final int n0 = objectsInUse(p, childPid, classToCheckForLeaks);
-        final int n1 = objectsInUse(p, childPid, classToCheckForLeaks);
+        final int n0 = objectsInUse(p, childPid, classNameRegex);
+        final int n1 = objectsInUse(p, childPid, classNameRegex);
         equal(p.waitFor(), 0);
         equal(p.exitValue(), 0);
         failed += p.exitValue();
@@ -226,7 +232,7 @@ public class TimedAcquireLeak {
         // implementation, and needing occasional adjustment.
         System.out.printf("%d -> %d%n", n0, n1);
         // Almost always n0 == n1
-        // Maximum jitter observed in practice is 10 -> 17
+        // Maximum jitter observed in practice is 7
         check(Math.abs(n1 - n0) < 10);
         check(n1 < 25);
         drainers.shutdown();
@@ -244,9 +250,9 @@ public class TimedAcquireLeak {
     // - in between calls to rendezvousChild, run code that may leak.
     //----------------------------------------------------------------
     public static class Job {
-        static String classToCheckForLeaks() {
+        static String classNameRegexToCheckForLeaks() {
             return
-                "java.util.concurrent.locks.AbstractQueuedSynchronizer$Node";
+                "\\Qjava.util.concurrent.locks.AbstractQueuedSynchronizer$\\E[A-Za-z]+";
         }
 
         public static void main(String[] args) throws Throwable {

test/jdk/java/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java

@@ -93,7 +93,7 @@ public final class CancelledLockLoops {
             barrier.await();
             if (print) {
                 long time = timer.getTime();
-                double secs = (double)(time) / 1000000000.0;
+                double secs = (double)time / 1000000000.0;
                 System.out.println("\t " + secs + "s run time");
             }
 

test/jdk/java/util/concurrent/locks/ReentrantLock/LockOncePerThreadLoops.java

@@ -94,7 +94,7 @@ public final class LockOncePerThreadLoops {
             barrier.await();
             if (print) {
                 long time = timer.getTime();
-                double secs = (double)(time) / 1000000000.0;
+                double secs = (double)time / 1000000000.0;
                 System.out.println("\t " + secs + "s run time");
             }
 

test/jdk/java/util/concurrent/locks/ReentrantLock/SimpleReentrantLockLoops.java

@@ -95,7 +95,7 @@ public final class SimpleReentrantLockLoops {
                 long time = timer.getTime();
                 long tpi = time / ((long)iters * nthreads);
                 System.out.print("\t" + LoopHelpers.rightJustify(tpi) + " ns per lock");
-                double secs = (double)(time) / 1000000000.0;
+                double secs = (double)time / 1000000000.0;
                 System.out.println("\t " + secs + "s run time");
             }
 

test/jdk/java/util/concurrent/locks/ReentrantLock/TimeoutLockLoops.java

@@ -96,7 +96,7 @@ public final class TimeoutLockLoops {
             barrier.await();
             if (print) {
                 long time = timer.getTime();
-                double secs = (double)(time) / 1000000000.0;
+                double secs = (double)time / 1000000000.0;
                 System.out.println("\t " + secs + "s run time");
             }
 

test/jdk/java/util/concurrent/locks/ReentrantReadWriteLock/MapLoops.java

@@ -91,8 +91,8 @@ public class MapLoops {
             premove = Integer.parseInt(args[4]);
 
         // normalize probabilities wrt random number generator
-        removesPerMaxRandom = (int)(((double)premove/100.0 * 0x7FFFFFFFL));
-        insertsPerMaxRandom = (int)(((double)pinsert/100.0 * 0x7FFFFFFFL));
+        removesPerMaxRandom = (int)((double)premove/100.0 * 0x7FFFFFFFL);
+        insertsPerMaxRandom = (int)((double)pinsert/100.0 * 0x7FFFFFFFL);
 
         System.out.println("Using " + mapClass.getName());
 
@@ -125,7 +125,7 @@ public class MapLoops {
             long time = timer.getTime();
             long tpo = time / (i * (long)nops);
             System.out.print(LoopHelpers.rightJustify(tpo) + " ns per op");
-            double secs = (double)(time) / 1000000000.0;
+            double secs = (double)time / 1000000000.0;
             System.out.println("\t " + secs + "s run time");
             map.clear();
         }