8264149: BreakpointInfo::set allocates metaspace object in VM thread

Reviewed-by: dholmes, iklam

src/hotspot/share/interpreter/interpreterRuntime.cpp

@@ -1115,13 +1115,7 @@ JRT_ENTRY(void, InterpreterRuntime::update_mdp_for_ret(JavaThread* thread, int r
 JRT_END
 
 JRT_ENTRY(MethodCounters*, InterpreterRuntime::build_method_counters(JavaThread* thread, Method* m))
-  MethodCounters* mcs = Method::build_method_counters(m, thread);
-  if (HAS_PENDING_EXCEPTION) {
-    // Only metaspace OOM is expected. No Java code executed.
-    assert((PENDING_EXCEPTION->is_a(vmClasses::OutOfMemoryError_klass())), "we expect only an OOM error here");
-    CLEAR_PENDING_EXCEPTION;
-  }
-  return mcs;
+  return Method::build_method_counters(thread, m);
 JRT_END
 
 

src/hotspot/share/interpreter/interpreterRuntime.hpp

@@ -158,7 +158,7 @@ class InterpreterRuntime: AllStatic {
 #ifdef ASSERT
   static void    verify_mdp(Method* method, address bcp, address mdp);
 #endif // ASSERT
-  static MethodCounters* build_method_counters(JavaThread* thread, Method* m);
+  static MethodCounters* build_method_counters(JavaThread* current, Method* m);
 };
 
 

src/hotspot/share/memory/allocation.cpp

@@ -79,10 +79,17 @@ void  StackObj::operator delete [](void* p)           { ShouldNotCallThis(); }
 void* MetaspaceObj::operator new(size_t size, ClassLoaderData* loader_data,
                                  size_t word_size,
                                  MetaspaceObj::Type type, TRAPS) throw() {
-  // Klass has it's own operator new
+  // Klass has its own operator new
   return Metaspace::allocate(loader_data, word_size, type, THREAD);
 }
 
+void* MetaspaceObj::operator new(size_t size, ClassLoaderData* loader_data,
+                                 size_t word_size,
+                                 MetaspaceObj::Type type) throw() {
+  assert(!Thread::current()->is_Java_thread(), "only allowed by non-Java thread");
+  return Metaspace::allocate(loader_data, word_size, type);
+}
+
 bool MetaspaceObj::is_valid(const MetaspaceObj* p) {
   // Weed out obvious bogus values first without traversing metaspace
   if ((size_t)p < os::min_page_size()) {

src/hotspot/share/memory/allocation.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -345,6 +345,9 @@ class MetaspaceObj {
                      size_t word_size,
                      Type type, Thread* thread) throw();
                      // can't use TRAPS from this header file.
+  void* operator new(size_t size, ClassLoaderData* loader_data,
+                     size_t word_size,
+                     Type type) throw();
   void operator delete(void* p) { ShouldNotCallThis(); }
 
   // Declare a *static* method with the same signature in any subclass of MetaspaceObj

src/hotspot/share/memory/metaspace.cpp

@@ -789,17 +789,14 @@ size_t Metaspace::max_allocation_word_size() {
   return metaspace::chunklevel::MAX_CHUNK_WORD_SIZE - max_overhead_words;
 }
 
+// This version of Metaspace::allocate does not throw OOM but simply returns NULL, and
+// is suitable for calling from non-Java threads.
+// Callers are responsible for checking null.
 MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
-                              MetaspaceObj::Type type, TRAPS) {
+                              MetaspaceObj::Type type) {
   assert(word_size <= Metaspace::max_allocation_word_size(),
          "allocation size too large (" SIZE_FORMAT ")", word_size);
   assert(!_frozen, "sanity");
-  assert(!(DumpSharedSpaces && THREAD->is_VM_thread()), "sanity");
-
-  if (HAS_PENDING_EXCEPTION) {
-    assert(false, "Should not allocate with exception pending");
-    return NULL;  // caller does a CHECK_NULL too
-  }
 
   assert(loader_data != NULL, "Should never pass around a NULL loader_data. "
         "ClassLoaderData::the_null_class_loader_data() should have been used.");
@@ -809,7 +806,30 @@ MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
   // Try to allocate metadata.
   MetaWord* result = loader_data->metaspace_non_null()->allocate(word_size, mdtype);
 
+  if (result != NULL) {
+    // Zero initialize.
+    Copy::fill_to_words((HeapWord*)result, word_size, 0);
+
+    log_trace(metaspace)("Metaspace::allocate: type %d return " PTR_FORMAT ".", (int)type, p2i(result));
+  }
+
+  return result;
+}
+
+MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
+                              MetaspaceObj::Type type, TRAPS) {
+
+  assert(THREAD->is_Java_thread(), "can't allocate in non-Java thread because we cannot throw exception");
+
+  if (HAS_PENDING_EXCEPTION) {
+    assert(false, "Should not allocate with exception pending");
+    return NULL;  // caller does a CHECK_NULL too
+  }
+
+  MetaWord* result = allocate(loader_data, word_size, type);
+
   if (result == NULL) {
+    MetadataType mdtype = (type == MetaspaceObj::ClassType) ? ClassType : NonClassType;
     tracer()->report_metaspace_allocation_failure(loader_data, word_size, type, mdtype);
 
     // Allocation failed.
@@ -819,18 +839,18 @@ MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
       // expansion of the metaspace.
       result = Universe::heap()->satisfy_failed_metadata_allocation(loader_data, word_size, mdtype);
     }
-  }
 
-  if (result == NULL) {
-    report_metadata_oome(loader_data, word_size, type, mdtype, THREAD);
-    assert(HAS_PENDING_EXCEPTION, "sanity");
-    return NULL;
-  }
+    if (result == NULL) {
+      report_metadata_oome(loader_data, word_size, type, mdtype, THREAD);
+      assert(HAS_PENDING_EXCEPTION, "sanity");
+      return NULL;
+    }
 
-  // Zero initialize.
-  Copy::fill_to_words((HeapWord*)result, word_size, 0);
+    // Zero initialize.
+    Copy::fill_to_words((HeapWord*)result, word_size, 0);
 
-  log_trace(metaspace)("Metaspace::allocate: type %d return " PTR_FORMAT ".", (int)type, p2i(result));
+    log_trace(metaspace)("Metaspace::allocate: type %d return " PTR_FORMAT ".", (int)type, p2i(result));
+  }
 
   return result;
 }

src/hotspot/share/memory/metaspace.hpp

@@ -124,6 +124,11 @@ class Metaspace : public AllStatic {
   static MetaWord* allocate(ClassLoaderData* loader_data, size_t word_size,
                             MetaspaceObj::Type type, TRAPS);
 
+  // Non-TRAPS version of allocate which can be called by a non-Java thread, that returns
+  // NULL on failure.
+  static MetaWord* allocate(ClassLoaderData* loader_data, size_t word_size,
+                            MetaspaceObj::Type type);
+
   static bool contains(const void* ptr);
   static bool contains_non_shared(const void* ptr);
 

src/hotspot/share/oops/method.cpp

@@ -557,25 +557,40 @@ void Method::build_interpreter_method_data(const methodHandle& method, TRAPS) {
   }
 }
 
-MethodCounters* Method::build_method_counters(Method* m, TRAPS) {
+MethodCounters* Method::build_method_counters(Thread* current, Method* m) {
   // Do not profile the method if metaspace has hit an OOM previously
   if (ClassLoaderDataGraph::has_metaspace_oom()) {
     return NULL;
   }
 
-  methodHandle mh(THREAD, m);
-  MethodCounters* counters = MethodCounters::allocate(mh, THREAD);
-  if (HAS_PENDING_EXCEPTION) {
+  methodHandle mh(current, m);
+  MethodCounters* counters;
+  if (current->is_Java_thread()) {
+    Thread* THREAD = current;
+    // Use the TRAPS version for a JavaThread so it will adjust the GC threshold
+    // if needed.
+    counters = MethodCounters::allocate_with_exception(mh, THREAD);
+    if (HAS_PENDING_EXCEPTION) {
+      CLEAR_PENDING_EXCEPTION;
+    }
+  } else {
+    // Call metaspace allocation that doesn't throw exception if the
+    // current thread isn't a JavaThread, ie. the VMThread.
+    counters = MethodCounters::allocate_no_exception(mh);
+  }
+
+  if (counters == NULL) {
     CompileBroker::log_metaspace_failure();
     ClassLoaderDataGraph::set_metaspace_oom(true);
-    return NULL;   // return the exception (which is cleared)
+    return NULL;
   }
+
   if (!mh->init_method_counters(counters)) {
     MetadataFactory::free_metadata(mh->method_holder()->class_loader_data(), counters);
   }
 
   if (LogTouchedMethods) {
-    mh->log_touched(CHECK_NULL);
+    mh->log_touched(current);
   }
 
   return mh->method_counters();
@@ -2374,7 +2389,7 @@ class TouchedMethodRecord : CHeapObj<mtTracing> {
 static const int TOUCHED_METHOD_TABLE_SIZE = 20011;
 static TouchedMethodRecord** _touched_method_table = NULL;
 
-void Method::log_touched(TRAPS) {
+void Method::log_touched(Thread* current) {
 
   const int table_size = TOUCHED_METHOD_TABLE_SIZE;
   Symbol* my_class = klass_name();
@@ -2386,7 +2401,7 @@ void Method::log_touched(TRAPS) {
                       my_sig->identity_hash();
   juint index = juint(hash) % table_size;
 
-  MutexLocker ml(THREAD, TouchedMethodLog_lock);
+  MutexLocker ml(current, TouchedMethodLog_lock);
   if (_touched_method_table == NULL) {
     _touched_method_table = NEW_C_HEAP_ARRAY2(TouchedMethodRecord*, table_size,
                                               mtTracing, CURRENT_PC);

src/hotspot/share/oops/method.hpp

@@ -228,28 +228,28 @@ class Method : public Metadata {
   void clear_all_breakpoints();
   // Tracking number of breakpoints, for fullspeed debugging.
   // Only mutated by VM thread.
-  u2   number_of_breakpoints()             const {
+  u2   number_of_breakpoints() const {
     MethodCounters* mcs = method_counters();
     if (mcs == NULL) {
       return 0;
     } else {
       return mcs->number_of_breakpoints();
     }
   }
-  void incr_number_of_breakpoints(TRAPS)         {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void incr_number_of_breakpoints(Thread* current) {
+    MethodCounters* mcs = get_method_counters(current);
     if (mcs != NULL) {
       mcs->incr_number_of_breakpoints();
     }
   }
-  void decr_number_of_breakpoints(TRAPS)         {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void decr_number_of_breakpoints(Thread* current) {
+    MethodCounters* mcs = get_method_counters(current);
     if (mcs != NULL) {
       mcs->decr_number_of_breakpoints();
     }
   }
   // Initialization only
-  void clear_number_of_breakpoints()             {
+  void clear_number_of_breakpoints() {
     MethodCounters* mcs = method_counters();
     if (mcs != NULL) {
       mcs->clear_number_of_breakpoints();
@@ -292,8 +292,8 @@ class Method : public Metadata {
 
 #if COMPILER2_OR_JVMCI
   // Count of times method was exited via exception while interpreting
-  void interpreter_throwout_increment(TRAPS) {
-    MethodCounters* mcs = get_method_counters(CHECK);
+  void interpreter_throwout_increment(Thread* current) {
+    MethodCounters* mcs = get_method_counters(current);
     if (mcs != NULL) {
       mcs->interpreter_throwout_increment();
     }
@@ -432,7 +432,7 @@ class Method : public Metadata {
 
   static void build_interpreter_method_data(const methodHandle& method, TRAPS);
 
-  static MethodCounters* build_method_counters(Method* m, TRAPS);
+  static MethodCounters* build_method_counters(Thread* current, Method* m);
 
   int interpreter_invocation_count()            { return invocation_count();          }
 
@@ -944,9 +944,9 @@ class Method : public Metadata {
   void print_made_not_compilable(int comp_level, bool is_osr, bool report, const char* reason);
 
  public:
-  MethodCounters* get_method_counters(TRAPS) {
+  MethodCounters* get_method_counters(Thread* current) {
     if (_method_counters == NULL) {
-      build_method_counters(this, CHECK_AND_CLEAR_NULL);
+      build_method_counters(current, this);
     }
     return _method_counters;
   }

src/hotspot/share/oops/methodCounters.cpp

@@ -55,7 +55,12 @@ MethodCounters::MethodCounters(const methodHandle& mh) :
   _backedge_mask = right_n_bits(CompilerConfig::scaled_freq_log(Tier0BackedgeNotifyFreqLog, scale)) << InvocationCounter::count_shift;
 }
 
-MethodCounters* MethodCounters::allocate(const methodHandle& mh, TRAPS) {
+MethodCounters* MethodCounters::allocate_no_exception(const methodHandle& mh) {
+  ClassLoaderData* loader_data = mh->method_holder()->class_loader_data();
+  return new(loader_data, method_counters_size(), MetaspaceObj::MethodCountersType) MethodCounters(mh);
+}
+
+MethodCounters* MethodCounters::allocate_with_exception(const methodHandle& mh, TRAPS) {
   ClassLoaderData* loader_data = mh->method_holder()->class_loader_data();
   return new(loader_data, method_counters_size(), MetaspaceObj::MethodCountersType, THREAD) MethodCounters(mh);
 }

src/hotspot/share/oops/methodCounters.hpp

@@ -69,7 +69,8 @@ class MethodCounters : public Metadata {
  public:
   virtual bool is_methodCounters() const { return true; }
 
-  static MethodCounters* allocate(const methodHandle& mh, TRAPS);
+  static MethodCounters* allocate_no_exception(const methodHandle& mh);
+  static MethodCounters* allocate_with_exception(const methodHandle& mh, TRAPS);
 
   void deallocate_contents(ClassLoaderData* loader_data) {}