Review comments

Author: stefank

test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObjectTest.java

@@ -23,10 +23,11 @@
  */
 
 /*
+ * @bug 8320515
  * @summary This test checks that ObjectMonitors with dead objects don't
             cause asserts, crashes, or failures when various sub-systems
             in the JVM find them.
- * @requires os.family != "windows"
+ * @requires os.family != "windows" & os.family != "aix"
  * @library /testlibrary /test/lib
  * @modules jdk.management
  */
@@ -42,25 +43,17 @@
  */
 
 /*
- * @test id=DumpThreadsAfterDetachBeforeJoin
+ * @test id=DumpThreadsAfterDetach
  * @run main/othervm/native MonitorWithDeadObjectTest 2
  */
 
-/*
- * @test id=DumpThreadsAfterDetachAfterJoin
- * @run main/othervm/native MonitorWithDeadObjectTest 3
- */
-
 import java.lang.management.ManagementFactory;
 import java.lang.management.ThreadMXBean;
 
 public class MonitorWithDeadObjectTest {
     public static native void createMonitorWithDeadObject();
-    public static native void createMonitorWithDeadObjectNoJoin();
     public static native void createMonitorWithDeadObjectDumpThreadsBeforeDetach();
 
-    public static native void joinTestThread();
-
     static {
         System.loadLibrary("MonitorWithDeadObjectTest");
     }
@@ -72,7 +65,8 @@ private static void dumpThreadsWithLockedMonitors() {
 
     private static void testDetachThread() {
         // Create an ObjectMonitor with a dead object from an
-        // attached thread.
+        // attached thread. This used to provoke an assert
+        // in DetachCurrentThread.
         createMonitorWithDeadObject();
     }
 
@@ -83,26 +77,13 @@ private static void testDumpThreadsBeforeDetach() {
         createMonitorWithDeadObjectDumpThreadsBeforeDetach();
     }
 
-    private static void testDumpThreadsAfterDetachBeforeJoin() {
-        createMonitorWithDeadObjectNoJoin();
-
-        // After createMonitorWithDeadObjectNoJoin has been called, there's an
-        // "owned" monitor with a dead object. The thread dumping code used to
-        // not tolerate such a monitor and would assert. Run a thread dump
-        // and make sure that it doesn't crash/assert.
-        dumpThreadsWithLockedMonitors();
-
-        joinTestThread();
-    }
-
-    private static void testDumpThreadsAfterDetachAfterJoin() {
-        createMonitorWithDeadObjectNoJoin();
-        joinTestThread();
+    private static void testDumpThreadsAfterDetach() {
+        createMonitorWithDeadObject();
 
-        // After createMonitorWithDeadObjectNoJoin has been called, there's an
-        // "owned" monitor with a dead object. The thread dumping code used to
-        // not tolerate such a monitor and would assert. Run a thread dump
-        // and make sure that it doesn't crash/assert.
+        // After createMonitorWithDeadObject has been called, there's an "owned"
+        // monitor with a dead object. The thread dumping code used to not
+        // tolerate such a monitor and would assert. Run a thread dump and make
+        // sure that it doesn't crash/assert.
         dumpThreadsWithLockedMonitors();
     }
 
@@ -111,8 +92,7 @@ public static void main(String[] args) throws Exception {
         switch (test) {
             case 0: testDetachThread(); break;
             case 1: testDumpThreadsBeforeDetach(); break;
-            case 2: testDumpThreadsAfterDetachBeforeJoin(); break;
-            case 3: testDumpThreadsAfterDetachAfterJoin(); break;
+            case 2: testDumpThreadsAfterDetach(); break;
             default: throw new RuntimeException("Unknown test");
         };
     }

test/hotspot/jtreg/runtime/Monitor/libMonitorWithDeadObjectTest.c

@@ -127,7 +127,6 @@ static void* create_monitor_with_dead_object_in_thread() {
   //   assert that we didn't have "owned" monitors with dead objects. This
   //   test provokes that situation and that asserts.
   if ((*jvm)->DetachCurrentThread(jvm) != JNI_OK) die("DetachCurrentThread");
-  pthread_exit(NULL);
 
   return NULL;
 }
@@ -146,45 +145,25 @@ static void* create_monitor_with_dead_object_and_dump_threads_in_thread() {
   thread_dump_with_locked_monitors(env);
 
   if ((*jvm)->DetachCurrentThread(jvm) != JNI_OK) die("DetachCurrentThread");
-  pthread_exit(NULL);
 
   return NULL;
 }
 
 JNIEXPORT void JNICALL Java_MonitorWithDeadObjectTest_createMonitorWithDeadObject(JNIEnv* env, jclass jc) {
-    pthread_attr_t attr;
     void* ret;
 
     (*env)->GetJavaVM(env, &jvm);
 
-    if (pthread_attr_init(&attr) != 0) die("pthread_attr_init");
-    if (pthread_create(&attacher, &attr, create_monitor_with_dead_object_in_thread, NULL) != 0) die("pthread_create");
+    if (pthread_create(&attacher, NULL, create_monitor_with_dead_object_in_thread, NULL) != 0) die("pthread_create");
     if (pthread_join(attacher, &ret) != 0) die("pthread_join");
 }
 
-JNIEXPORT void JNICALL Java_MonitorWithDeadObjectTest_createMonitorWithDeadObjectNoJoin(JNIEnv* env, jclass jc) {
-    pthread_attr_t attr;
-    void* ret;
-
-    (*env)->GetJavaVM(env, &jvm);
-
-    if (pthread_attr_init(&attr) != 0) die("pthread_attr_init");
-    if (pthread_create(&attacher, &attr, create_monitor_with_dead_object_in_thread, NULL) != 0) die("pthread_create");
-}
-
 JNIEXPORT void JNICALL Java_MonitorWithDeadObjectTest_createMonitorWithDeadObjectDumpThreadsBeforeDetach(JNIEnv* env, jclass jc) {
-    pthread_attr_t attr;
     void* ret;
 
     (*env)->GetJavaVM(env, &jvm);
 
-    if (pthread_attr_init(&attr) != 0) die("pthread_attr_init");
-    if (pthread_create(&attacher, &attr, create_monitor_with_dead_object_and_dump_threads_in_thread, NULL) != 0) die("pthread_create");
-    if (pthread_join(attacher, &ret) != 0) die("pthread_join");
-}
-
-JNIEXPORT void JNICALL Java_MonitorWithDeadObjectTest_joinTestThread(JNIEnv* env, jclass jc) {
-    void* ret;
+    if (pthread_create(&attacher, NULL, create_monitor_with_dead_object_and_dump_threads_in_thread, NULL) != 0) die("pthread_create");
     if (pthread_join(attacher, &ret) != 0) die("pthread_join");
 }
 

test/hotspot/jtreg/serviceability/jvmti/GetOwnedMonitorInfo/GetOwnedMonitorInfoTest.java

@@ -55,7 +55,7 @@ private static void jniMonitorEnterAndLetObjectDie() {
         // assert when an owned monitor with a dead object was found.
         // Inject this situation into this test that performs other
         // GetOwnedMonitorInfo testing.
-        Object obj = new Object() { public String toString() {return "";} };
+        Object obj = new Object() {};
         jniMonitorEnter(obj);
         if (!Thread.holdsLock(obj)) {
             throw new RuntimeException("The object is not locked");
@@ -76,6 +76,8 @@ public static void runTest(boolean isVirtual, boolean jni) throws Exception {
         final GetOwnedMonitorInfoTest lock = new GetOwnedMonitorInfoTest();
 
         Thread t1 = threadFactory.newThread(() -> {
+            Thread.currentThread().setName("Worker-Thread");
+
             if (jni) {
                 jniMonitorEnterAndLetObjectDie();
             }

test/hotspot/jtreg/serviceability/jvmti/GetOwnedMonitorInfo/libGetOwnedMonitorInfoTest.c

@@ -22,6 +22,7 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include "jvmti.h"
 #include "jni.h"
@@ -268,6 +269,7 @@ JNIEXPORT void JNICALL
 Java_GetOwnedMonitorInfoTest_jniMonitorEnter(JNIEnv* env, jclass cls, jobject obj) {
     if ((*env)->MonitorEnter(env, obj) != 0) {
         fprintf(stderr, "MonitorEnter failed");
+        exit(-1);
     }
 }