8202073: MetaspaceAllocationTest gtest shall lock during space creation

tstuefe · tstuefe · commit 2329ce7e2dd2 · 2018-04-23T16:25:16.000+02:00
Reviewed-by: coleenp
diff --git a/src/hotspot/share/memory/metaspace.cpp b/src/hotspot/share/memory/metaspace.cpp
@@ -3378,6 +3378,7 @@ void SpaceManager::track_metaspace_memory_usage() {
 }
 
 MetaWord* SpaceManager::grow_and_allocate(size_t word_size) {
+  assert_lock_strong(_lock);
   assert(vs_list()->current_virtual_space() != NULL,
          "Should have been set");
   assert(current_chunk() == NULL ||
@@ -3553,6 +3554,7 @@ void SpaceManager::deallocate(MetaWord* p, size_t word_size) {
 // Adds a chunk to the list of chunks in use.
 void SpaceManager::add_chunk(Metachunk* new_chunk, bool make_current) {
 
+  assert_lock_strong(_lock);
   assert(new_chunk != NULL, "Should not be NULL");
   assert(new_chunk->next() == NULL, "Should not be on a list");
 
diff --git a/test/hotspot/gtest/memory/test_metaspace_allocation.cpp b/test/hotspot/gtest/memory/test_metaspace_allocation.cpp
@@ -26,6 +26,7 @@
 #include "memory/allocation.inline.hpp"
 #include "memory/metaspace.hpp"
 #include "runtime/mutex.hpp"
+#include "runtime/mutexLocker.hpp"
 #include "runtime/os.hpp"
 #include "utilities/align.hpp"
 #include "utilities/debug.hpp"
@@ -104,7 +105,12 @@ class MetaspaceAllocationTest : public ::testing::Test {
     // Let every ~10th space be an anonymous one to test different allocation patterns.
     const Metaspace::MetaspaceType msType = (os::random() % 100 < 10) ?
       Metaspace::AnonymousMetaspaceType : Metaspace::StandardMetaspaceType;
-    _spaces[i].space = new ClassLoaderMetaspace(_spaces[i].lock, msType);
+    {
+      // Pull lock during space creation, since this is what happens in the VM too
+      // (see ClassLoaderData::metaspace_non_null(), which we mimick here).
+      MutexLockerEx ml(_spaces[i].lock,  Mutex::_no_safepoint_check_flag);
+      _spaces[i].space = new ClassLoaderMetaspace(_spaces[i].lock, msType);
+    }
     _spaces[i].allocated = 0;
     ASSERT_TRUE(_spaces[i].space != NULL);
   }
@@ -171,6 +177,7 @@ class MetaspaceAllocationTest : public ::testing::Test {
             } else {
               size = os::random() % 64;
             }
+            // Note: In contrast to space creation, no need to lock here. ClassLoaderMetaspace::allocate() will lock itself.
             MetaWord* const p = _spaces[index].space->allocate(size, mdType);
             if (p == NULL) {
               // We very probably did hit the metaspace "until-gc" limit.
@@ -196,6 +203,7 @@ class MetaspaceAllocationTest : public ::testing::Test {
           force_switch = true;
         } else {
           assert(_spaces[index].space != NULL && _spaces[index].allocated > 0, "Sanity");
+          // Note: do not lock here. In the "wild" (the VM), we do not so either (see ~ClassLoaderData()).
           delete _spaces[index].space;
           _spaces[index].space = NULL;
           _spaces[index].allocated = 0;
