8369622: GlobalChunkPoolMutex is recursively locked during error handling

src/hotspot/share/memory/arena.cpp

@@ -39,19 +39,26 @@
 // It is used very early in the vm initialization, in allocation
 // code and other areas.  For many calls, the current thread has not
 // been created so we cannot use Mutex.
-static PlatformMutex* GlobalChunkPoolMutex = nullptr;
+static DeferredStatic<PlatformMutex> GlobalChunkPoolMutex;
 
 void Arena::initialize_chunk_pool() {
-  GlobalChunkPoolMutex = new PlatformMutex();
+  GlobalChunkPoolMutex.initialize();
 }
 
-ChunkPoolLocker::ChunkPoolLocker() {
-  assert(GlobalChunkPoolMutex != nullptr, "must be initialized");
-  GlobalChunkPoolMutex->lock();
+ChunkPoolLocker::ChunkPoolLocker(LockStrategy ls) {
+  if (ls == LockStrategy::Lock) {
+    GlobalChunkPoolMutex->lock();
+    _locked = true;
+  } else {
+    assert(ls == LockStrategy::Try, "must be");
+    _locked = GlobalChunkPoolMutex->try_lock();
+  }
 };
 
 ChunkPoolLocker::~ChunkPoolLocker() {
-  GlobalChunkPoolMutex->unlock();
+  if (_locked) {
+    GlobalChunkPoolMutex->unlock();
+  }
 };
 
 // Pre-defined default chunk sizes must be arena-aligned, see Chunk::operator new()

src/hotspot/share/memory/arena.hpp

@@ -38,8 +38,11 @@
 #define ARENA_ALIGN(x) (align_up((x), ARENA_AMALLOC_ALIGNMENT))
 
 class ChunkPoolLocker : public StackObj {
+  bool _locked;
  public:
-  ChunkPoolLocker();
+  enum class LockStrategy { Lock, Try };
+
+  ChunkPoolLocker(LockStrategy ls = LockStrategy::Lock);
   ~ChunkPoolLocker();
 };
 

src/hotspot/share/nmt/mallocTracker.cpp

@@ -65,7 +65,11 @@ void MallocMemorySnapshot::copy_to(MallocMemorySnapshot* s) {
   // Use lock to make sure that mtChunks don't get deallocated while the
   // copy is going on, because their size is adjusted using this
   // buffer in make_adjustment().
-  ChunkPoolLocker lock;
+  ChunkPoolLocker::LockStrategy ls = ChunkPoolLocker::LockStrategy::Lock;
+  if (VMError::is_error_reported() && VMError::is_error_reported_in_current_thread()) {
+    ls = ChunkPoolLocker::LockStrategy::Try;
+  }
+  ChunkPoolLocker cpl(ls);
   s->_all_mallocs = _all_mallocs;
   size_t total_size = 0;
   size_t total_count = 0;

src/hotspot/share/nmt/nmtUsage.cpp

@@ -30,6 +30,7 @@
 #include "nmt/nmtUsage.hpp"
 #include "nmt/threadStackTracker.hpp"
 #include "runtime/mutexLocker.hpp"
+#include "utilities/vmError.hpp"
 
 // Enabled all options for snapshot.
 const NMTUsageOptions NMTUsage::OptionsAll = { true, true, true };
@@ -58,7 +59,11 @@ void NMTUsage::update_malloc_usage() {
   // Lock needed to keep values in sync, total area size
   // is deducted from mtChunk in the end to give correct values.
   {
-    ChunkPoolLocker lock;
+    ChunkPoolLocker::LockStrategy ls = ChunkPoolLocker::LockStrategy::Lock;
+    if (VMError::is_error_reported() && VMError::is_error_reported_in_current_thread()) {
+      ls = ChunkPoolLocker::LockStrategy::Try;
+    }
+    ChunkPoolLocker cpl(ls);
     ms = MallocMemorySummary::as_snapshot();
   }
 

test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "memory/allocation.hpp"
+#include "memory/arena.hpp"
 #include "nmt/memTracker.hpp"
 #include "runtime/os.hpp"
 #include "sanitizers/address.hpp"
@@ -142,6 +143,21 @@ DEFINE_TEST(test_corruption_on_realloc_growing, COMMON_NMT_HEAP_CORRUPTION_MESSA
 static void test_corruption_on_realloc_shrinking()  { test_corruption_on_realloc(0x11, 0x10); }
 DEFINE_TEST(test_corruption_on_realloc_shrinking, COMMON_NMT_HEAP_CORRUPTION_MESSAGE_PREFIX);
 
+static void test_chunkpool_lock() {
+  if (!MemTracker::enabled()) {
+    tty->print_cr("Skipped");
+    return;
+  }
+  PrintNMTStatistics = true;
+  {
+    ChunkPoolLocker cpl;
+    char* mem = (char*)os::malloc(100, mtTest);
+    memset(mem - 16, 0, 100 + 16 + 2);
+    os::free(mem);
+  }
+}
+DEFINE_TEST(test_chunkpool_lock, COMMON_NMT_HEAP_CORRUPTION_MESSAGE_PREFIX);
+
 ///////
 
 // realloc is the trickiest of the bunch. Test that realloc works and correctly takes over