8290020: Deadlock in leakprofiler::emit_events during shutdown

Reviewed-by: mgronlun, dholmes, egahlin

Author: LudwikJaniuk

src/hotspot/share/jfr/jfr.cpp

@@ -116,8 +116,8 @@ void Jfr::on_resolution(const Parse* parse, const ciKlass* holder, const ciMetho
 }
 #endif
 
-void Jfr::on_vm_shutdown(bool exception_handler) {
-  if (JfrRecorder::is_recording()) {
+void Jfr::on_vm_shutdown(bool exception_handler, bool halt) {
+  if (!halt && JfrRecorder::is_recording()) {
     JfrEmergencyDump::on_vm_shutdown(exception_handler);
   }
 }

src/hotspot/share/jfr/jfr.hpp

@@ -65,7 +65,7 @@ class Jfr : AllStatic {
   static void on_resolution(const GraphBuilder* builder, const ciKlass* holder, const ciMethod* target);
   static void on_java_thread_start(JavaThread* starter, JavaThread* startee);
   static void on_set_current_thread(JavaThread* jt, oop thread);
-  static void on_vm_shutdown(bool exception_handler = false);
+  static void on_vm_shutdown(bool exception_handler = false, bool halt = false);
   static void on_vm_error_report(outputStream* st);
   static bool on_flight_recorder_option(const JavaVMOption** option, char* delimiter);
   static bool on_start_flight_recording_option(const JavaVMOption** option, char* delimiter);

src/hotspot/share/prims/jvm.cpp

@@ -441,7 +441,7 @@ JVM_END
 
 
 JVM_ENTRY_NO_ENV(void, JVM_Halt(jint code))
-  before_exit(thread);
+  before_exit(thread, true);
   vm_exit(code);
 JVM_END
 

src/hotspot/share/runtime/java.cpp

@@ -401,7 +401,7 @@ void print_statistics() {
 // Note: before_exit() can be executed only once, if more than one threads
 //       are trying to shutdown the VM at the same time, only one thread
 //       can run before_exit() and all other threads must wait.
-void before_exit(JavaThread* thread) {
+void before_exit(JavaThread* thread, bool halt) {
   #define BEFORE_EXIT_NOT_RUN 0
   #define BEFORE_EXIT_RUNNING 1
   #define BEFORE_EXIT_DONE    2
@@ -448,7 +448,7 @@ void before_exit(JavaThread* thread) {
     event.commit();
   }
 
-  JFR_ONLY(Jfr::on_vm_shutdown();)
+  JFR_ONLY(Jfr::on_vm_shutdown(false, halt);)
 
   // Stop the WatcherThread. We do this before disenrolling various
   // PeriodicTasks to reduce the likelihood of races.

src/hotspot/share/runtime/java.hpp

@@ -33,7 +33,7 @@ class JavaThread;
 class Symbol;
 
 // Execute code before all handles are released and thread is killed; prologue to vm_exit
-extern void before_exit(JavaThread * thread);
+extern void before_exit(JavaThread * thread, bool halt = false);
 
 // Forced VM exit (i.e, internal error or JVM_Exit)
 extern void vm_exit(int code);

test/jdk/jdk/jfr/jvm/TestDumpOnCrash.java

@@ -76,41 +76,33 @@ public static void main(String[] args) throws Exception {
 
     public static void main(String[] args) throws Exception {
         // Test without dumppath
-        test(CrasherIllegalAccess.class, "", true);
-        test(CrasherIllegalAccess.class, "", false);
-        test(CrasherHalt.class, "", true);
-        test(CrasherHalt.class, "", false);
+        test(CrasherIllegalAccess.class, "", true, null, true);
+        test(CrasherIllegalAccess.class, "", false, null, true);
+
+        // JDK-8290020 disables dumps when calling halt, so expect no dump.
+        test(CrasherHalt.class, "", true, null, false);
+        test(CrasherHalt.class, "", false, null, false);
 
         // Test with dumppath
         Path dumppath = Files.createTempDirectory(null);
         try {
-            test(CrasherIllegalAccess.class, "", true, dumppath.toString());
-            test(CrasherIllegalAccess.class, "", false, dumppath.toString());
-            test(CrasherHalt.class, "", true, dumppath.toString());
-            test(CrasherHalt.class, "", false, dumppath.toString());
+            test(CrasherIllegalAccess.class, "", true, dumppath.toString(), true);
+            test(CrasherIllegalAccess.class, "", false, dumppath.toString(), true);
         } finally {
             dumppath.toFile().delete();
         }
 
         // Test is excluded until 8219680 is fixed
         // @ignore 8219680
-        // test(CrasherSig.class, "FPE", true);
-    }
-
-    private static void test(Class<?> crasher, String signal, boolean disk) throws Exception {
-        test(crasher, signal, disk, null);
+        // test(CrasherSig.class, "FPE", true, true);
     }
 
-    private static void test(Class<?> crasher, String signal, boolean disk, String dumppath) throws Exception {
-        test(crasher, signal, disk, dumppath, dumppath);
-    }
-
-    private static void test(Class<?> crasher, String signal, boolean disk, String dumppath, String expectedPath) throws Exception {
+    private static void test(Class<?> crasher, String signal, boolean disk, String dumppath, boolean expectDump) throws Exception {
         // The JVM may be in a state it can't recover from, so try three times
         // before concluding functionality is not working.
         for (int attempt = 0; attempt < ATTEMPTS; attempt++) {
             try {
-                verify(runProcess(crasher, signal, disk, dumppath), expectedPath);
+                verify(runProcess(crasher, signal, disk, dumppath), dumppath, expectDump);
                 return;
             } catch (Exception e) {
                 System.out.println("Attempt " + attempt + ". Verification failed:");
@@ -148,19 +140,23 @@ private static long runProcess(Class<?> crasher, String signal, boolean disk, St
         return p.pid();
     }
 
-    private static void verify(long pid, String dumppath) throws IOException {
+    private static void verify(long pid, String dumppath, boolean expectDump) throws IOException {
         String fileName = "hs_err_pid" + pid + ".jfr";
         Path file = (dumppath == null) ? Paths.get(fileName) : Paths.get(dumppath, fileName);
         file = file.toAbsolutePath().normalize();
 
-        Asserts.assertTrue(Files.exists(file), "No emergency jfr recording file " + file + " exists");
-        Asserts.assertNotEquals(Files.size(file), 0L, "File length 0. Should at least be some bytes");
-        System.out.printf("File size=%d%n", Files.size(file));
+        if (expectDump) {
+            Asserts.assertTrue(Files.exists(file), "No emergency jfr recording file " + file + " exists");
+            Asserts.assertNotEquals(Files.size(file), 0L, "File length 0. Should at least be some bytes");
+            System.out.printf("File size=%d%n", Files.size(file));
 
-        List<RecordedEvent> events = RecordingFile.readAllEvents(file);
-        Asserts.assertFalse(events.isEmpty(), "No event found");
-        System.out.printf("Found event %s%n", events.get(0).getEventType().getName());
+            List<RecordedEvent> events = RecordingFile.readAllEvents(file);
+            Asserts.assertFalse(events.isEmpty(), "No event found");
+            System.out.printf("Found event %s%n", events.get(0).getEventType().getName());
 
-        Files.delete(file);
+            Files.delete(file);
+        } else {
+            Asserts.assertFalse(Files.exists(file), "Emergency jfr recording file " + file + " exists but wasn't expected");
+        }
     }
 }