8274794: Print all owned locks in hs_err file

src/hotspot/share/runtime/mutex.cpp

@@ -31,6 +31,7 @@
 #include "runtime/osThread.hpp"
 #include "runtime/safepointMechanism.inline.hpp"
 #include "runtime/thread.inline.hpp"
+#include "runtime/threadCritical.hpp"
 #include "utilities/events.hpp"
 #include "utilities/macros.hpp"
 
@@ -267,7 +268,36 @@ bool Monitor::wait(int64_t timeout) {
   return wait_status != 0;          // return true IFF timeout
 }
 
+// Array used to print owned locks on error.
+static Mutex* _mutex_array = NULL;
+
+void Mutex::add_to_global_list() {
+  // Add mutex to print_owned_locks_on_error array
+  ThreadCritical tc;
+  Mutex* next = _mutex_array;
+  _next_mutex = next;
+  _prev_mutex = nullptr;
+  _mutex_array = this;
+  if (next != nullptr) {
+    next->_prev_mutex = this;
+  }
+}
+
+void Mutex::remove_from_global_list() {
+  // Remove mutex from print_owned_locks_on_error array
+  ThreadCritical tc;
+  Mutex* old_next = _next_mutex;
+  assert(old_next != nullptr, "this list can never be empty");
+  old_next->_prev_mutex = _prev_mutex;
+  if (_prev_mutex == nullptr) {
+    _mutex_array = old_next;
+  } else {
+    _prev_mutex->_next_mutex = old_next;
+  }
+}
+
 Mutex::~Mutex() {
+  remove_from_global_list();
   assert_owner(NULL);
   os::free(const_cast<char*>(_name));
 }
@@ -280,6 +310,8 @@ Mutex::Mutex(Rank rank, const char * name, bool allow_vm_block) : _owner(NULL) {
   _allow_vm_block  = allow_vm_block;
   _rank            = rank;
   _skip_rank_check = false;
+  _next            = nullptr;
+  _last_owner      = nullptr;
 
   assert(_rank >= static_cast<Rank>(0) && _rank <= safepoint, "Bad lock rank %s: %s", rank_name(), name);
 
@@ -288,16 +320,51 @@ Mutex::Mutex(Rank rank, const char * name, bool allow_vm_block) : _owner(NULL) {
   assert(_rank > nosafepoint || _allow_vm_block,
          "Locks that don't check for safepoint should always allow the vm to block: %s", name);
 #endif
+  add_to_global_list();
 }
 
 bool Mutex::owned_by_self() const {
   return owner() == Thread::current();
 }
 
-void Mutex::print_on_error(outputStream* st) const {
+void Mutex::print_on(outputStream* st) const {
   st->print("[" PTR_FORMAT, p2i(this));
   st->print("] %s", _name);
   st->print(" - owner thread: " PTR_FORMAT, p2i(owner()));
+#ifdef ASSERT
+  if (_allow_vm_block) {
+    st->print_raw(" allow_vm_block");
+  }
+  st->print(" %s", rank_name());
+#endif
+  st->cr();
+}
+
+// Print all mutexes/monitors that are currently owned by a thread; called
+// by fatal error handler.
+// This function doesn't take the ThreadCritical lock to avoid potential
+// deadlock during error reporting.
+void Mutex::print_owned_locks_on_error(outputStream* st) {
+  assert(VMError::is_error_reported(), "should only be called during error reporting");
+  ResourceMark rm;
+  st->print("VM Mutexes/Monitors currently owned by a thread: ");
+  bool none = true;
+  Mutex *m = _mutex_array;
+  int array_count = 0;
+  while (m != nullptr) {
+     array_count++;
+     // see if it has an owner
+     if (m->owner() != NULL) {
+       if (none) {
+          st->cr();
+          none = false;
+       }
+       m->print_on(st);
+     }
+     m = m->_next_mutex;
+  }
+  if (none) st->print_cr("None");
+  st->print_cr("Total Mutex count %d", array_count);
 }
 
 // ----------------------------------------------------------------------------------
@@ -343,21 +410,7 @@ void Mutex::assert_no_overlap(Rank orig, Rank adjusted, int adjust) {
            rank_name_internal(orig), adjust, rank_name_internal(adjusted));
   }
 }
-#endif // ASSERT
-
-#ifndef PRODUCT
-void Mutex::print_on(outputStream* st) const {
-  st->print("Mutex: [" PTR_FORMAT "] %s - owner: " PTR_FORMAT,
-            p2i(this), _name, p2i(owner()));
-  if (_allow_vm_block) {
-    st->print("%s", " allow_vm_block");
-  }
-  DEBUG_ONLY(st->print(" %s", rank_name()));
-  st->cr();
-}
-#endif // PRODUCT
 
-#ifdef ASSERT
 void Mutex::assert_owner(Thread * expected) {
   const char* msg = "invalid owner";
   if (expected == NULL) {

src/hotspot/share/runtime/mutex.hpp

@@ -88,19 +88,24 @@ class Mutex : public CHeapObj<mtSynchronizer> {
   Thread* volatile _owner;
   void raw_set_owner(Thread* new_owner) { Atomic::store(&_owner, new_owner); }
 
+  // Embed pointers for mutex array for error reporting.
+  Mutex* _next_mutex;
+  Mutex* _prev_mutex;
+
+  void add_to_global_list();
+  void remove_from_global_list();
+
  protected:                              // Monitor-Mutex metadata
   os::PlatformMonitor _lock;             // Native monitor implementation
   const char* _name;                     // Name of mutex/monitor
 
-  // Debugging fields for naming, deadlock detection, etc. (some only used in debug mode)
-#ifndef PRODUCT
-  bool    _allow_vm_block;
-#endif
+  // Debugging fields for naming, deadlock detection, etc.
 #ifdef ASSERT
   Rank    _rank;                 // rank (to avoid/detect potential deadlocks)
   Mutex*  _next;                 // Used by a Thread to link up owned locks
   Thread* _last_owner;           // the last thread to own the lock
-  bool _skip_rank_check;         // read only by owner when doing rank checks
+  bool    _skip_rank_check;      // read only by owner when doing rank checks
+  bool    _allow_vm_block;
 
   static bool contains(Mutex* locks, Mutex* lock);
   static Mutex* get_least_ranked_lock(Mutex* locks);
@@ -189,11 +194,11 @@ class Mutex : public CHeapObj<mtSynchronizer> {
 
   const char *name() const                  { return _name; }
 
-  void print_on_error(outputStream* st) const;
-  #ifndef PRODUCT
-    void print_on(outputStream* st) const;
-    void print() const                      { print_on(::tty); }
-  #endif
+  // Print all mutexes/monitors that are currently owned by a thread; called
+  // by fatal error handler.
+  static void print_owned_locks_on_error(outputStream* st);
+  void print_on(outputStream* st) const;
+  void print() const { print_on(::tty); }
 };
 
 class Monitor : public Mutex {

src/hotspot/share/runtime/mutexLocker.cpp

@@ -157,11 +157,6 @@ Mutex*   Bootclasspath_lock           = NULL;
 Monitor* JVMCI_lock                   = NULL;
 #endif
 
-
-#define MAX_NUM_MUTEX 128
-static Mutex* _mutex_array[MAX_NUM_MUTEX];
-static int _num_mutex;
-
 #ifdef ASSERT
 void assert_locked_or_safepoint(const Mutex* lock) {
   // check if this thread owns the lock (common case)
@@ -194,26 +189,18 @@ void assert_locked_or_safepoint_or_handshake(const Mutex* lock, const JavaThread
 }
 #endif
 
-static void add_mutex(Mutex* var) {
-  assert(_num_mutex < MAX_NUM_MUTEX, "increase MAX_NUM_MUTEX");
-  _mutex_array[_num_mutex++] = var;
-}
-
 #define def(var, type, pri, vm_block) {       \
   var = new type(Mutex::pri, #var, vm_block); \
-  add_mutex(var);                             \
 }
 
 // Specify relative ranked lock
 #ifdef ASSERT
 #define defl(var, type, held_lock, vm_block) {         \
   var = new type(held_lock->rank()-1, #var, vm_block); \
-  add_mutex(var);                                      \
 }
 #else
 #define defl(var, type, held_lock, vm_block) {         \
   var = new type(Mutex::safepoint, #var, vm_block);    \
-  add_mutex(var);                                      \
 }
 #endif
 
@@ -380,23 +367,3 @@ GCMutexLocker::GCMutexLocker(Mutex* mutex) {
     _mutex->lock();
   }
 }
-
-// Print all mutexes/monitors that are currently owned by a thread; called
-// by fatal error handler.
-void print_owned_locks_on_error(outputStream* st) {
-  st->print("VM Mutex/Monitor currently owned by a thread: ");
-  bool none = true;
-  for (int i = 0; i < _num_mutex; i++) {
-     // see if it has an owner
-     if (_mutex_array[i]->owner() != NULL) {
-       if (none) {
-          // print format used by Mutex::print_on_error()
-          st->print_cr(" ([mutex/lock_event])");
-          none = false;
-       }
-       _mutex_array[i]->print_on_error(st);
-       st->cr();
-     }
-  }
-  if (none) st->print_cr("None");
-}

src/hotspot/share/runtime/mutexLocker.hpp

@@ -169,12 +169,6 @@ extern Mutex* tty_lock;                          // lock to synchronize output.
 // order.  If their implementations change such that these assumptions
 // are violated, a whole lot of code will break.
 
-// Print all mutexes/monitors that are currently owned by a thread; called
-// by fatal error handler.
-void print_owned_locks_on_error(outputStream* st);
-
-char *lock_name(Mutex *mutex);
-
 // for debugging: check that we're already owning this lock (or are at a safepoint / handshake)
 #ifdef ASSERT
 void assert_locked_or_safepoint(const Mutex* lock);

src/hotspot/share/utilities/vmError.cpp

@@ -1015,7 +1015,7 @@ void VMError::report(outputStream* st, bool _verbose) {
 
      // mutexes/monitors that currently have an owner
      if (_verbose) {
-       print_owned_locks_on_error(st);
+       Mutex::print_owned_locks_on_error(st);
        st->cr();
      }
 
@@ -1913,6 +1913,12 @@ void VMError::controlled_crash(int how) {
   switch (how) {
     case 1: assert(how == 0, "test assert"); break;
     case 2: guarantee(how == 0, "test guarantee"); break;
+    case 3: {
+      Mutex* ErrorTest_lock = new Mutex(Mutex::nosafepoint, "ErrorTest_lock");
+      MutexLocker ml(ErrorTest_lock, Mutex::_no_safepoint_check_flag);
+      assert(how == 0, "test assert with lock");
+      break;
+    }
 
     // The other cases are unused.
     case 14: crash_with_segfault(); break;

test/hotspot/jtreg/runtime/ErrorHandling/ErrorFileOverwriteTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2019, SAP. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -42,46 +42,6 @@
 
 public class ErrorFileOverwriteTest {
 
-  private static File findHsErrorFileInOutput(OutputAnalyzer output) {
-
-    String hs_err_file = output.firstMatch("# *(\\S*hs_err_pid.*\\.log)", 1);
-    if(hs_err_file ==null) {
-      throw new RuntimeException("Did not find hs-err file in output.\n");
-    }
-
-    File f = new File(hs_err_file);
-    if (!f.exists()) {
-      throw new RuntimeException("hs-err file missing at "
-              + f.getAbsolutePath() + ".\n");
-    }
-
-    return f;
-
-  }
-
-  private static void scanHsErrorFileForContent(File f, Pattern[] pattern) throws IOException {
-    FileInputStream fis = new FileInputStream(f);
-    BufferedReader br = new BufferedReader(new InputStreamReader(fis));
-    String line = null;
-
-    int currentPattern = 0;
-
-    String lastLine = null;
-    while ((line = br.readLine()) != null && currentPattern < pattern.length) {
-      if (pattern[currentPattern].matcher(line).matches()) {
-        System.out.println("Found: " + line + ".");
-        currentPattern++;
-      }
-      lastLine = line;
-    }
-    br.close();
-
-    if (currentPattern < pattern.length) {
-      throw new RuntimeException("hs-err file incomplete (first missing pattern: " +  pattern[currentPattern] + ")");
-    }
-
-  }
-
   public static void do_test(boolean with_percent_p) throws Exception {
 
     // Crash twice.
@@ -110,10 +70,10 @@ public static void do_test(boolean with_percent_p) throws Exception {
     output_detail.shouldMatch("# " + errorFileStem + ".*");
     System.out.println("First crash: Found expected output on tty. Ok.");
 
-    File f = findHsErrorFileInOutput(output_detail);
+    File f = ErrorFileScanner.findHsErrorFileInOutput(output_detail);
     System.out.println("First crash: Found hs error file at " + f.getAbsolutePath());
 
-    scanHsErrorFileForContent(f, new Pattern[] {
+    ErrorFileScanner.scanHsErrorFileForContent(f, new Pattern[] {
             Pattern.compile("# *Internal Error.*"),
             Pattern.compile("Command Line:.*-XX:ErrorHandlerTest=1.*-XX:ErrorFile=" + errorFileStem + ".*")
     });
@@ -136,7 +96,7 @@ public static void do_test(boolean with_percent_p) throws Exception {
     output_detail.shouldMatch("# " + errorFileStem + ".*");
     System.out.println("Second crash: Found expected output on tty. Ok.");
 
-    File f2 = findHsErrorFileInOutput(output_detail);
+    File f2 = ErrorFileScanner.findHsErrorFileInOutput(output_detail);
     System.out.println("Second crash: Found hs error file at " + f2.getAbsolutePath());
 
     if (with_percent_p) {
@@ -145,7 +105,7 @@ public static void do_test(boolean with_percent_p) throws Exception {
       }
     }
 
-    scanHsErrorFileForContent(f2, new Pattern[] {
+    ErrorFileScanner.scanHsErrorFileForContent(f2, new Pattern[] {
             Pattern.compile("# *Internal Error.*"),
             Pattern.compile("Command Line:.*-XX:ErrorHandlerTest=2.*-XX:ErrorFile=" + errorFileStem + ".*")
     });