src/hotspot/share/jfr/recorder/repository/jfrEmergencyDump.cpp
changeset 58863 c16ac7a2eba4
parent 58503 726a3945e934
child 58945 a3b046720c3b
--- a/src/hotspot/share/jfr/recorder/repository/jfrEmergencyDump.cpp	Wed Oct 30 16:14:56 2019 +0100
+++ b/src/hotspot/share/jfr/recorder/repository/jfrEmergencyDump.cpp	Wed Oct 30 19:43:52 2019 +0100
@@ -248,7 +248,6 @@
 }
 
 static const char* create_emergency_dump_path() {
-  assert(JfrStream_lock->owned_by_self(), "invariant");
   char* buffer = NEW_RESOURCE_ARRAY_RETURN_NULL(char, JVM_MAXPATHLEN);
   if (NULL == buffer) {
     return NULL;
@@ -291,7 +290,6 @@
 // Caller needs ResourceMark
 static const char* create_emergency_chunk_path(const char* repository_path) {
   assert(repository_path != NULL, "invariant");
-  assert(JfrStream_lock->owned_by_self(), "invariant");
   const size_t repository_path_len = strlen(repository_path);
   // date time
   char date_time_buffer[32] = { 0 };
@@ -307,12 +305,11 @@
     return NULL;
   }
   // append the individual substrings
-  jio_snprintf(chunk_path, chunkname_max_len, "%s%s%s%s", repository_path_len, os::file_separator(), date_time_buffer, chunk_file_jfr_ext);
+  jio_snprintf(chunk_path, chunkname_max_len, "%s%s%s%s", repository_path, os::file_separator(), date_time_buffer, chunk_file_jfr_ext);
   return chunk_path;
 }
 
 static fio_fd emergency_dump_file_descriptor() {
-  assert(JfrStream_lock->owned_by_self(), "invariant");
   ResourceMark rm;
   const char* const emergency_dump_path = create_emergency_dump_path();
   return emergency_dump_path != NULL ? open_exclusivly(emergency_dump_path) : invalid_fd;
@@ -325,7 +322,6 @@
 void JfrEmergencyDump::on_vm_error(const char* repository_path) {
   assert(repository_path != NULL, "invariant");
   ResourceMark rm;
-  MutexLocker stream_lock(JfrStream_lock, Mutex::_no_safepoint_check_flag);
   const fio_fd emergency_fd = emergency_dump_file_descriptor();
   if (emergency_fd != invalid_fd) {
     RepositoryIterator iterator(repository_path, strlen(repository_path));
@@ -340,17 +336,25 @@
 *
 * 1. if the thread state is not "_thread_in_vm", we will quick transition
 *    it to "_thread_in_vm".
-* 2. the nesting state for both resource and handle areas are unknown,
-*    so we allocate new fresh arenas, discarding the old ones.
-* 3. if the thread is the owner of some critical lock(s), unlock them.
+* 2. if the thread is the owner of some critical lock(s), unlock them.
 *
 * If we end up deadlocking in the attempt of dumping out jfr data,
 * we rely on the WatcherThread task "is_error_reported()",
-* to exit the VM after a hard-coded timeout.
+* to exit the VM after a hard-coded timeout (disallow WatcherThread to emergency dump).
 * This "safety net" somewhat explains the aggressiveness in this attempt.
 *
 */
-static void prepare_for_emergency_dump(Thread* thread) {
+static bool prepare_for_emergency_dump() {
+  if (JfrStream_lock->owned_by_self()) {
+    // crashed during jfr rotation, disallow recursion
+    return false;
+  }
+  Thread* const thread = Thread::current();
+  if (thread->is_Watcher_thread()) {
+    // need WatcherThread as a safeguard against potential deadlocks
+    return false;
+  }
+
   if (thread->is_Java_thread()) {
     ((JavaThread*)thread)->set_thread_state(_thread_in_vm);
   }
@@ -388,7 +392,6 @@
     VMOperationRequest_lock->unlock();
   }
 
-
   if (Service_lock->owned_by_self()) {
     Service_lock->unlock();
   }
@@ -413,13 +416,10 @@
     JfrBuffer_lock->unlock();
   }
 
-  if (JfrStream_lock->owned_by_self()) {
-    JfrStream_lock->unlock();
-  }
-
   if (JfrStacktrace_lock->owned_by_self()) {
     JfrStacktrace_lock->unlock();
   }
+  return true;
 }
 
 static volatile int jfr_shutdown_lock = 0;
@@ -429,24 +429,9 @@
 }
 
 void JfrEmergencyDump::on_vm_shutdown(bool exception_handler) {
-  if (!guard_reentrancy()) {
+  if (!(guard_reentrancy() && prepare_for_emergency_dump())) {
     return;
   }
-  // function made non-reentrant
-  Thread* thread = Thread::current();
-  if (exception_handler) {
-    // we are crashing
-    if (thread->is_Watcher_thread()) {
-      // The Watcher thread runs the periodic thread sampling task.
-      // If it has crashed, it is likely that another thread is
-      // left in a suspended state. This would mean the system
-      // will not be able to ever move to a safepoint. We try
-      // to avoid issuing safepoint operations when attempting
-      // an emergency dump, but a safepoint might be already pending.
-      return;
-    }
-    prepare_for_emergency_dump(thread);
-  }
   EventDumpReason event;
   if (event.should_commit()) {
     event.set_reason(exception_handler ? "Crash" : "Out of Memory");
@@ -458,8 +443,6 @@
     LeakProfiler::emit_events(max_jlong, false);
   }
   const int messages = MSGBIT(MSG_VM_ERROR);
-  ResourceMark rm(thread);
-  HandleMark hm(thread);
   JfrRecorderService service;
   service.rotate(messages);
 }