8050486: compiler/rtm/ tests fail due to monitor deflation at safepoint synchronization

Reviewed-by: kvn, iignatyev

test/compiler/rtm/locking/TestRTMAbortRatio.java

@@ -126,10 +126,7 @@ public class TestRTMAbortRatio extends CommandLineOptionTest {
 
         @Override
         public String[] getMethodsToCompileNames() {
-            return new String[] {
-                    getMethodWithLockName(),
-                    Unsafe.class.getName() + "::addressSize"
-            };
+            return new String[] { getMethodWithLockName() };
         }
 
         public void lock(boolean abort) {
@@ -147,10 +144,12 @@ public class TestRTMAbortRatio extends CommandLineOptionTest {
         public static void main(String args[]) throws Throwable {
             Asserts.assertGTE(args.length, 1, "One argument required.");
             Test t = new Test();
-            if (Boolean.valueOf(args[0])) {
+            boolean shouldBeInflated = Boolean.valueOf(args[0]);
+            if (shouldBeInflated) {
                 AbortProvoker.inflateMonitor(t.monitor);
             }
             for (int i = 0; i < Test.TOTAL_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated);
                 t.lock(i >= Test.WARMUP_ITERATIONS);
             }
         }

test/compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java

@@ -156,10 +156,7 @@ public class TestRTMAfterNonRTMDeopt extends CommandLineOptionTest {
 
         @Override
         public String[] getMethodsToCompileNames() {
-            return new String[] {
-                getMethodWithLockName(),
-                sun.misc.Unsafe.class.getName() + "::forceAbort"
-            };
+            return new String[] { getMethodWithLockName() };
         }
 
         public void forceAbort(int a[], boolean abort) {
@@ -182,13 +179,15 @@ public class TestRTMAfterNonRTMDeopt extends CommandLineOptionTest {
         public static void main(String args[]) throws Throwable {
             Test t = new Test();
 
-            if (Boolean.valueOf(args[0])) {
+            boolean shouldBeInflated = Boolean.valueOf(args[0]);
+            if (shouldBeInflated) {
                 AbortProvoker.inflateMonitor(t.monitor);
             }
 
             int tmp[] = new int[1];
 
             for (int i = 0; i < Test.ITERATIONS; i++ ) {
+                AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated);
                 if (i == Test.RANGE_CHECK_AT) {
                     t.forceAbort(new int[0], false);
                 } else {

test/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java

@@ -129,10 +129,7 @@ public class TestRTMDeoptOnLowAbortRatio extends CommandLineOptionTest {
 
         @Override
         public String[] getMethodsToCompileNames() {
-            return new String[] {
-                getMethodWithLockName(),
-                sun.misc.Unsafe.class.getName() + "::addressSize"
-            };
+            return new String[] { getMethodWithLockName() };
         }
 
         public void forceAbort(boolean abort) {
@@ -150,11 +147,12 @@ public class TestRTMDeoptOnLowAbortRatio extends CommandLineOptionTest {
         public static void main(String args[]) throws Throwable {
             Asserts.assertGTE(args.length, 1, "One argument required.");
             Test t = new Test();
-
-            if (Boolean.valueOf(args[0])) {
+            boolean shouldBeInflated = Boolean.valueOf(args[0]);
+            if (shouldBeInflated) {
                 AbortProvoker.inflateMonitor(t.monitor);
             }
             for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated);
                 t.forceAbort(
                         i == TestRTMDeoptOnLowAbortRatio.LOCKING_THRESHOLD);
             }

test/compiler/rtm/locking/TestRTMLockingThreshold.java

@@ -142,10 +142,7 @@ public class TestRTMLockingThreshold extends CommandLineOptionTest {
 
         @Override
         public String[] getMethodsToCompileNames() {
-            return new String[] {
-                getMethodWithLockName(),
-                sun.misc.Unsafe.class.getName() + "::addressSize"
-            };
+            return new String[] { getMethodWithLockName() };
         }
 
         public void lock(boolean abort) {
@@ -163,11 +160,12 @@ public class TestRTMLockingThreshold extends CommandLineOptionTest {
         public static void main(String args[]) throws Throwable {
             Asserts.assertGTE(args.length, 1, "One argument required.");
             Test t = new Test();
-
-            if (Boolean.valueOf(args[0])) {
+            boolean shouldBeInflated = Boolean.valueOf(args[0]);
+            if (shouldBeInflated) {
                 AbortProvoker.inflateMonitor(t.monitor);
             }
             for (int i = 0; i < Test.TOTAL_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated);
                 t.lock(i % 2 == 1);
             }
         }

test/compiler/rtm/locking/TestRTMTotalCountIncrRate.java

@@ -116,9 +116,7 @@ public class TestRTMTotalCountIncrRate extends CommandLineOptionTest {
 
         @Override
         public String[] getMethodsToCompileNames() {
-            return new String[] {
-                getMethodWithLockName()
-            };
+            return new String[] { getMethodWithLockName() };
         }
 
         public void lock() {
@@ -134,11 +132,13 @@ public class TestRTMTotalCountIncrRate extends CommandLineOptionTest {
         public static void main(String args[]) throws Throwable {
             Asserts.assertGTE(args.length, 1, "One argument required.");
             Test test = new Test();
-
-            if (Boolean.valueOf(args[0])) {
+            boolean shouldBeInflated = Boolean.valueOf(args[0]);
+            if (shouldBeInflated) {
                 AbortProvoker.inflateMonitor(test.monitor);
             }
             for (long i = 0L; i < Test.TOTAL_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(test.monitor,
+                        shouldBeInflated);
                 test.lock();
             }
         }

test/compiler/rtm/locking/TestUseRTMAfterLockInflation.java

@@ -51,7 +51,7 @@ import rtm.predicate.SupportedVM;
  * Compiled method invoked {@code AbortProvoker.DEFAULT_ITERATIONS} times before
  * lock inflation and the same amount of times after inflation.
  * As a result total locks count should be equal to
- * {@code 2*AbortProvoker.DEFAULT_ITERATIONS}.
+ * {@code 2 * AbortProvoker.DEFAULT_ITERATIONS}.
  * It is a pretty strict assertion which could fail if some retriable abort
  * happened: it could be {@code AbortType.RETRIABLE} or
  * {@code AbortType.MEM_CONFLICT}, but unfortunately abort can has both these
@@ -100,7 +100,6 @@ public class TestUseRTMAfterLockInflation extends CommandLineOptionTest {
     }
 
     public static class Test {
-
         /**
          * Usage:
          * Test <provoker type>
@@ -112,10 +111,12 @@ public class TestUseRTMAfterLockInflation extends CommandLineOptionTest {
             AbortProvoker provoker
                     = AbortType.lookup(Integer.valueOf(args[0])).provoker();
             for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(provoker, false /*deflated*/);
                 provoker.forceAbort();
             }
             provoker.inflateMonitor();
             for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) {
+                AbortProvoker.verifyMonitorState(provoker, true /*inflated*/);
                 provoker.forceAbort();
             }
         }

test/compiler/testlibrary/rtm/AbortProvoker.java

@@ -29,8 +29,7 @@ import java.util.concurrent.BrokenBarrierException;
 import java.util.concurrent.CyclicBarrier;
 
 import com.oracle.java.testlibrary.Asserts;
-import com.oracle.java.testlibrary.Utils;
-import sun.misc.Unsafe;
+import sun.hotspot.WhiteBox;
 
 /**
  * Base class for different transactional execution abortion
@@ -38,6 +37,9 @@ import sun.misc.Unsafe;
  */
 public abstract class AbortProvoker implements CompilableTest {
     public static final long DEFAULT_ITERATIONS = 10000L;
+    private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();
+    @SuppressWarnings("unused")
+    private static int sharedState = 0;
     /**
      * Inflates monitor associated with object {@code monitor}.
      * Inflation is forced by entering the same monitor from
@@ -48,36 +50,76 @@ public abstract class AbortProvoker implements CompilableTest {
      * @throws Exception if something went wrong.
      */
     public static Object inflateMonitor(Object monitor) throws Exception {
-        Unsafe unsafe = Utils.getUnsafe();
         CyclicBarrier barrier = new CyclicBarrier(2);
 
         Runnable inflatingRunnable = () -> {
-            unsafe.monitorEnter(monitor);
-            try {
-                barrier.await();
-                barrier.await();
-            } catch (InterruptedException | BrokenBarrierException e) {
-                throw new RuntimeException(
-                        "Synchronization issue occurred.", e);
-            } finally {
-                unsafe.monitorExit(monitor);
+            synchronized (monitor) {
+                try {
+                    barrier.await();
+                } catch (BrokenBarrierException  | InterruptedException e) {
+                    throw new RuntimeException(
+                            "Synchronization issue occurred.", e);
+                }
+                try {
+                    monitor.wait();
+                } catch (InterruptedException e) {
+                    throw new AssertionError("The thread waiting on an"
+                            + " inflated monitor was interrupted, thus test"
+                            + " results may be incorrect.", e);
+                }
             }
         };
 
         Thread t = new Thread(inflatingRunnable);
+        t.setDaemon(true);
         t.start();
         // Wait until thread t enters the monitor.
         barrier.await();
-        // At this point monitor will be owned by thread t,
-        // so our attempt to enter the same monitor will force
-        // monitor inflation.
-        Asserts.assertFalse(unsafe.tryMonitorEnter(monitor),
-                            "Not supposed to enter the monitor first");
-        barrier.await();
-        t.join();
+        synchronized (monitor) {
+            // At this point thread t is already waiting on the monitor.
+            // Modifying static field just to avoid lock's elimination.
+            sharedState++;
+        }
+        verifyMonitorState(monitor, true /* inflated */);
         return monitor;
     }
 
+    /**
+     * Verifies that {@code monitor} is a stack-lock or inflated lock depending
+     * on {@code shouldBeInflated} value. If {@code monitor} is inflated while
+     * it is expected that it should be a stack-lock, then this method attempts
+     * to deflate it by forcing a safepoint and then verifies the state once
+     * again.
+     *
+     * @param monitor monitor to be verified.
+     * @param shouldBeInflated flag indicating whether or not monitor is
+     *                         expected to be inflated.
+     * @throws RuntimeException if the {@code monitor} in a wrong state.
+     */
+    public static void verifyMonitorState(Object monitor,
+            boolean shouldBeInflated) {
+        if (!shouldBeInflated && WHITE_BOX.isMonitorInflated(monitor)) {
+            WHITE_BOX.forceSafepoint();
+        }
+        Asserts.assertEQ(WHITE_BOX.isMonitorInflated(monitor), shouldBeInflated,
+                "Monitor in a wrong state.");
+    }
+    /**
+     * Verifies that monitor used by the {@code provoker} is a stack-lock or
+     * inflated lock depending on {@code shouldBeInflated} value. If such
+     * monitor is inflated while it is expected that it should be a stack-lock,
+     * then this method attempts to deflate it by forcing a safepoint and then
+     * verifies the state once again.
+     *
+     * @param provoker AbortProvoker whose monitor's state should be verified.
+     * @param shouldBeInflated flag indicating whether or not monitor is
+     *                         expected to be inflated.
+     * @throws RuntimeException if the {@code monitor} in a wrong state.
+     */
+    public static void verifyMonitorState(AbortProvoker provoker,
+            boolean shouldBeInflated) {
+        verifyMonitorState(provoker.monitor, shouldBeInflated);
+    }
 
     /**
      * Get instance of specified AbortProvoker, inflate associated monitor
@@ -120,6 +162,7 @@ public abstract class AbortProvoker implements CompilableTest {
         }
 
         for (long i = 0; i < iterations; i++) {
+            AbortProvoker.verifyMonitorState(provoker, monitorShouldBeInflated);
             provoker.forceAbort();
         }
     }

test/compiler/testlibrary/rtm/BusyLock.java

@@ -77,7 +77,7 @@ public class BusyLock implements CompilableTest, Runnable {
         }
     }
 
-    public void test() {
+    public void syncAndTest() {
         try {
             barrier.await();
             // wait until monitor is locked by a ::run method
@@ -85,6 +85,10 @@ public class BusyLock implements CompilableTest, Runnable {
         } catch (InterruptedException | BrokenBarrierException e) {
             throw new RuntimeException("Synchronization error happened.", e);
         }
+        test();
+    }
+
+    public void test() {
         synchronized(monitor) {
             BusyLock.field++;
         }
@@ -130,7 +134,7 @@ public class BusyLock implements CompilableTest, Runnable {
 
         Thread t = new Thread(busyLock);
         t.start();
-        busyLock.test();
+        busyLock.syncAndTest();
         t.join();
     }
 }

test/compiler/testlibrary/rtm/MemoryConflictProvoker.java

@@ -69,11 +69,6 @@ class MemoryConflictProvoker extends AbortProvoker {
      * Accesses and modifies memory region from within the transaction.
      */
     public void transactionalRegion() {
-        try {
-            barrier.await();
-        } catch (InterruptedException | BrokenBarrierException e) {
-            throw new RuntimeException(e);
-        }
         for (int i = 0; i < MemoryConflictProvoker.INNER_ITERATIONS; i++) {
             synchronized(monitor) {
                 MemoryConflictProvoker.field--;
@@ -86,6 +81,11 @@ class MemoryConflictProvoker extends AbortProvoker {
         try {
             Thread t = new Thread(conflictingThread);
             t.start();
+            try {
+                barrier.await();
+            } catch (InterruptedException | BrokenBarrierException e) {
+                throw new RuntimeException(e);
+            }
             transactionalRegion();
             t.join();
         } catch (Exception e) {

test/compiler/testlibrary/rtm/RTMTestBase.java

@@ -238,10 +238,10 @@ public class RTMTestBase {
         String[] filteredVMOpts = Utils.getFilteredTestJavaOpts(filters);
         Collections.addAll(finalVMOpts, filteredVMOpts);
         Collections.addAll(finalVMOpts, "-Xcomp", "-server",
-                "-XX:-TieredCompilation",
+                "-XX:-TieredCompilation", "-XX:+UseRTMLocking",
                 CommandLineOptionTest.UNLOCK_DIAGNOSTIC_VM_OPTIONS,
                 CommandLineOptionTest.UNLOCK_EXPERIMENTAL_VM_OPTIONS,
-                "-XX:+UseRTMLocking");
+                "-Xbootclasspath/a:.", "-XX:+WhiteBoxAPI");
 
         if (test != null) {
             for (String method : test.getMethodsToCompileNames()) {