8031701: java/lang/management/ThreadMXBean/Locks.java: Thread WaitingThread is...

8031701: java/lang/management/ThreadMXBean/Locks.java: Thread WaitingThread is expected to wait on Object but got null Thread.State = RUNNABLE
Reviewed-by: mchung, dsamersoff

test/java/lang/management/ThreadMXBean/Locks.java

 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,18 +27,19 @@
  * @summary Basic unit test of ThreadInfo.getLockName()
  *          and ThreadInfo.getLockOwnerName()
  * @author  Mandy Chung
+ * @author  Jaroslav Bachorik
  *
- * @build ThreadExecutionSynchronizer
  * @run main/othervm Locks
  */
 
 import java.lang.management.*;
+import java.util.concurrent.Phaser;
 
 public class Locks {
-    private static Object objA = new Object();
-    private static Object objB = new Object();
-    private static Object objC = new Object();
-    private static ThreadMXBean tm = ManagementFactory.getThreadMXBean();
+    private static final Object objA = new Object();
+    private static final Object objB = new Object();
+    private static final Object objC = new Object();
+    private static final ThreadMXBean tm = ManagementFactory.getThreadMXBean();
 
     private static boolean testFailed = false;
 
@@ -49,15 +50,27 @@ public class Locks {
                 Integer.toHexString(System.identityHashCode(lock));
     }
 
+    private static void assertNoLock(Thread t) {
+        long tid = t.getId();
+        ThreadInfo info = tm.getThreadInfo(tid);
+        String result = info.getLockName();
+
+        if (result != null) {
+            throw new RuntimeException("Thread " + t.getName() + " is not supposed to hold any lock. " +
+                                       "Currently owning lock: " + result);
+        }
+    }
+
     private static void checkBlockedObject(Thread t, Object lock, Thread owner,
                                            Thread.State expectedState) {
-        ThreadInfo info = tm.getThreadInfo(t.getId());
+        long tid = t.getId();
+        ThreadInfo info = tm.getThreadInfo(tid);
         String result = info.getLockName();
         String expectedLock = (lock != null ? getLockName(lock) : null);
         String expectedOwner = (owner != null ? owner.getName() : null);
 
         if (lock != null) {
-            if (expectedState ==Thread.State.BLOCKED) {
+            if (expectedState == Thread.State.BLOCKED) {
                 int retryCount=0;
                 while(info.getThreadState() != Thread.State.BLOCKED) {
                     if (retryCount++ > 500) {
@@ -67,6 +80,8 @@ public class Locks {
                                 " Thread.State = " + info.getThreadState());
                     }
                     goSleep(100);
+                    info = tm.getThreadInfo(tid);
+                    result = info.getLockName();
                 }
             }
             if (expectedState == Thread.State.WAITING &&
@@ -100,53 +115,49 @@ public class Locks {
         }
     }
 
-    static ThreadExecutionSynchronizer thrsync = new ThreadExecutionSynchronizer();
-    static ThreadExecutionSynchronizer thrsync1 = new ThreadExecutionSynchronizer();
+    private static volatile int dummyCounter = 0;
 
     static class LockAThread extends Thread {
-        public LockAThread() {
+        private final Phaser p;
+        public LockAThread(Phaser p) {
             super("LockAThread");
+            this.p = p;
         }
         public void run() {
             synchronized(objA) {
                 // stop here  for LockBThread to hold objB
-               thrsync.waitForSignal();
-
                 System.out.println("LockAThread about to block on objB");
-               synchronized(objB) {};
+                p.arriveAndAwaitAdvance(); // Phase 1 (blocking)
+                synchronized(objB) {
+                    dummyCounter++;
+                };
             }
+            p.arriveAndAwaitAdvance(); // Phase 2 (blocking)
             System.out.println("LockAThread about to exit");
-            // The state could be anything. The expected state value
-            // passed with this method is not verified.
-            checkBlockedObject(this, null, null, Thread.State.TERMINATED);
+            // Make sure the current thread is not holding any lock
+            assertNoLock(this);
         }
     }
 
     static class LockBThread extends Thread {
-        public LockBThread() {
+        private final Phaser p;
+        public LockBThread(Phaser p) {
             super("LockBThread");
+            this.p = p;
         }
         public void run() {
             synchronized(objB) {
-               // signal waiting LockAThread.
-               thrsync.signal();
-
                 System.out.println("LockBThread about to block on objC");
+                p.arriveAndAwaitAdvance(); // Phase 1 (blocking)
                 // Signal main thread about to block on objC
-               thrsync1.signal();
-               synchronized(objC) {};
+                synchronized(objC) {
+                    dummyCounter++;
+                };
             }
+            p.arriveAndAwaitAdvance(); // Phase 2 (blocking)
             System.out.println("LockBThread about to exit");
-            // The state could be anything. The expected state value
-            // passed with this method is not verified.
-            checkBlockedObject(this, null, null, Thread.State.TERMINATED);
-        }
-
-        public void aboutToLockC() {
-            // Stop here till LockBThread about to blocked
-            // for lock objC.
-            thrsync1.waitForSignal();
-            goSleep(500);
+            // Make sure the current thread is not holding any lock
+            assertNoLock(this);
         }
     }
 
@@ -154,15 +165,17 @@ public class Locks {
     private static Object ready = new Object();
     private static CheckerThread checker;
     static class WaitingThread extends Thread {
-        public WaitingThread() {
+        private final Phaser p;
+        public WaitingThread(Phaser p) {
             super("WaitingThread");
+            this.p = p;
         }
         public void run() {
             synchronized(objC) {
                 System.out.println("WaitingThread about to wait on objC");
                 try {
                     // Signal checker thread, about to wait on objC.
-                   thrsync.signal();
+                    p.arriveAndAwaitAdvance(); // Phase 1 (waiting)
                     objC.wait();
                 } catch (InterruptedException e) {
                     e.printStackTrace();
@@ -173,13 +186,15 @@ public class Locks {
                 System.out.println("WaitingThread about to block on ready");
                 // signal checker thread that it is about acquire
                 // object ready.
-               thrsync.signal();
-               synchronized(ready) {};
+                p.arriveAndAwaitAdvance(); // Phase 2 (waiting)
+                synchronized(ready) {
+                    dummyCounter++;
+                };
             }
             synchronized(objC) {
                 try {
                     // signal checker thread, about to wait on objC
-                    thrsync.signal();
+                    p.arriveAndAwaitAdvance(); // Phase 3 (waiting)
                     objC.wait();
                 } catch (InterruptedException e) {
                     e.printStackTrace();
@@ -190,13 +205,15 @@ public class Locks {
         }
     }
     static class CheckerThread extends Thread {
-        public CheckerThread() {
+        private final Phaser p;
+        public CheckerThread(Phaser p) {
             super("CheckerThread");
+            this.p = p;
         }
 
         private void waitForState(Thread.State state) {
-            thrsync.waitForSignal();
-            while (waiter.getState() != state) {
+            p.arriveAndAwaitAdvance();
+            while (!waiter.isInterrupted() && waiter.getState() != state) {
                 goSleep(10);
             }
         }
@@ -204,7 +221,7 @@ public class Locks {
         public void run() {
             synchronized (ready) {
                 // wait until WaitingThread about to wait for objC
-                waitForState(Thread.State.WAITING);
+                waitForState(Thread.State.WAITING); // Phase 1 (waiting)
                 checkBlockedObject(waiter, objC, null, Thread.State.WAITING);
 
                 synchronized (objC) {
@@ -213,13 +230,13 @@ public class Locks {
 
                 // wait for waiter thread to about to enter
                 // synchronized object ready.
-                waitForState(Thread.State.BLOCKED);
+                waitForState(Thread.State.BLOCKED); // Phase 2 (waiting)
                 checkBlockedObject(waiter, ready, this, Thread.State.BLOCKED);
             }
 
             // wait for signal from waiting thread that it is about
             // wait for objC.
-            waitForState(Thread.State.WAITING);
+            waitForState(Thread.State.WAITING); // Phase 3 (waiting)
             synchronized(objC) {
                 checkBlockedObject(waiter, objC, Thread.currentThread(), Thread.State.WAITING);
                 objC.notify();
@@ -235,24 +252,24 @@ public class Locks {
         LockAThread t1;
         LockBThread t2;
 
+        Phaser p = new Phaser(3);
         synchronized(objC) {
-            // The state could be anything. The expected state value
-            // passed with this method is not verified.
-            checkBlockedObject(mainThread, null, null, Thread.State.RUNNABLE);
+            // Make sure the main thread is not holding any lock
+            assertNoLock(mainThread);
 
             // Test deadlock case
             // t1 holds lockA and attempts to lock B
             // t2 holds lockB and attempts to lock C
-            t1 = new LockAThread();
+
+            t1 = new LockAThread(p);
             t1.start();
 
-            t2 = new LockBThread();
+            t2 = new LockBThread(p);
             t2.start();
 
-            t2.aboutToLockC();
-
-            checkBlockedObject(t1, objB, t2, Thread.State.BLOCKED);
+            p.arriveAndAwaitAdvance(); // Phase 1 (blocking)
             checkBlockedObject(t2, objC, mainThread, Thread.State.BLOCKED);
+            checkBlockedObject(t1, objB, t2, Thread.State.BLOCKED);
 
             long[] expectedThreads = new long[3];
             expectedThreads[0] = t1.getId(); // blocked on lockB
@@ -260,13 +277,14 @@ public class Locks {
             expectedThreads[2] = mainThread.getId(); // owner of lockC
             findThreadsBlockedOn(objB, expectedThreads);
         }
-        goSleep(100);
+        p.arriveAndAwaitAdvance(); // Phase 2 (blocking)
 
+        p = new Phaser(2);
         // Test Object.wait() case
-        waiter = new WaitingThread();
+        waiter = new WaitingThread(p);
         waiter.start();
 
-        checker = new CheckerThread();
+        checker = new CheckerThread(p);
         checker.start();
 
         try {

test/java/lang/management/ThreadMXBean/ThreadExecutionSynchronizer.java

-/*
- * Copyright (c) 2004, 2013, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-/*
- *
- * @summary This class is used to synchronize execution of two threads.
- * @author  Swamy Venkataramanappa
- */
-
-import java.util.concurrent.Semaphore;
-
-public class ThreadExecutionSynchronizer {
-
-    private volatile boolean waiting;
-    private final Semaphore semaphore;
-
-    public ThreadExecutionSynchronizer() {
-        semaphore = new Semaphore(1);
-        waiting = false;
-    }
-
-    // Synchronizes two threads execution points.
-    // Basically any thread could get scheduled to run and
-    // it is not possible to know which thread reaches expected
-    // execution point. So whichever thread reaches a execution
-    // point first wait for the second thread. When the second thread
-    // reaches the expected execution point will wake up
-    // the thread which is waiting here.
-    void stopOrGo() {
-        semaphore.acquireUninterruptibly(); // Thread can get blocked.
-        if (!waiting) {
-            waiting = true;
-            // Wait for second thread to enter this method.
-            while(!semaphore.hasQueuedThreads()) {
-                try {
-                    Thread.sleep(20);
-                } catch (InterruptedException xx) {}
-            }
-            semaphore.release();
-        } else {
-            waiting = false;
-            semaphore.release();
-        }
-    }
-
-    // Wrapper function just for code readability.
-    void waitForSignal() {
-        stopOrGo();
-        goSleep(50);
-    }
-
-    void signal() {
-        stopOrGo();
-        goSleep(50);
-    }
-
-    private static void goSleep(long ms) {
-        try {
-            Thread.sleep(ms);
-        } catch (InterruptedException e) {
-            e.printStackTrace();
-            System.out.println("Unexpected exception.");
-        }
-    }
-}