7012505: BreakpointWithFullGC.sh fails with Internal Error (src/share/vm/oops/methodOop.cpp:220)

Summary: Rebuild breakpoint cache at gc_epilogue instead of during oops_do
Reviewed-by: dcubed, ysr, coleenp

src/share/vm/ci/ciEnv.cpp

@@ -409,15 +409,15 @@ ciKlass* ciEnv::get_klass_by_name_impl(ciKlass* accessing_klass,
   } else {
     fail_type = _unloaded_ciinstance_klass;
   }
-  klassOop found_klass;
+  KlassHandle found_klass;
   if (!require_local) {
-    found_klass =
-      SystemDictionary::find_constrained_instance_or_array_klass(sym, loader,
-                                                                 KILL_COMPILE_ON_FATAL_(fail_type));
+    klassOop kls = SystemDictionary::find_constrained_instance_or_array_klass(
+        sym, loader, KILL_COMPILE_ON_FATAL_(fail_type));
+    found_klass = KlassHandle(THREAD, kls);
   } else {
-    found_klass =
-      SystemDictionary::find_instance_or_array_klass(sym, loader, domain,
-                                                     KILL_COMPILE_ON_FATAL_(fail_type));
+    klassOop kls = SystemDictionary::find_instance_or_array_klass(
+        sym, loader, domain, KILL_COMPILE_ON_FATAL_(fail_type));
+    found_klass = KlassHandle(THREAD, kls);
   }
 
   // If we fail to find an array klass, look again for its element type.
@@ -444,9 +444,9 @@ ciKlass* ciEnv::get_klass_by_name_impl(ciKlass* accessing_klass,
     }
   }
 
-  if (found_klass != NULL) {
+  if (found_klass() != NULL) {
     // Found it.  Build a CI handle.
-    return get_object(found_klass)->as_klass();
+    return get_object(found_klass())->as_klass();
   }
 
   if (require_local)  return NULL;

src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp

@@ -4979,6 +4979,7 @@ void CMSCollector::checkpointRootsFinalWork(bool asynch,
   if (should_unload_classes()) {
     CodeCache::gc_epilogue();
   }
+  JvmtiExport::gc_epilogue();
 
   // If we encountered any (marking stack / work queue) overflow
   // events during the current CMS cycle, take appropriate

src/share/vm/gc_implementation/g1/g1MarkSweep.cpp

@@ -113,6 +113,7 @@ void G1MarkSweep::invoke_at_safepoint(ReferenceProcessor* rp,
 
   Threads::gc_epilogue();
   CodeCache::gc_epilogue();
+  JvmtiExport::gc_epilogue();
 
   // refs processing: clean slate
   GenMarkSweep::_ref_processor = NULL;

src/share/vm/gc_implementation/parallelScavenge/psMarkSweep.cpp

@@ -258,6 +258,7 @@ void PSMarkSweep::invoke_no_policy(bool clear_all_softrefs) {
     BiasedLocking::restore_marks();
     Threads::gc_epilogue();
     CodeCache::gc_epilogue();
+    JvmtiExport::gc_epilogue();
 
     COMPILER2_PRESENT(DerivedPointerTable::update_pointers());
 

src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp

@@ -1054,6 +1054,7 @@ void PSParallelCompact::post_compact()
 
   Threads::gc_epilogue();
   CodeCache::gc_epilogue();
+  JvmtiExport::gc_epilogue();
 
   COMPILER2_PRESENT(DerivedPointerTable::update_pointers());
 

src/share/vm/memory/genMarkSweep.cpp

@@ -158,6 +158,7 @@ void GenMarkSweep::invoke_at_safepoint(int level, ReferenceProcessor* rp,
 
   Threads::gc_epilogue();
   CodeCache::gc_epilogue();
+  JvmtiExport::gc_epilogue();
 
   if (PrintGC && !PrintGCDetails) {
     gch->print_heap_change(gch_prev_used);

src/share/vm/prims/jvmtiExport.cpp

@@ -2268,6 +2268,14 @@ void JvmtiExport::oops_do(OopClosure* f) {
   JvmtiVMObjectAllocEventCollector::oops_do_for_all_threads(f);
 }
 
+void JvmtiExport::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* f) {
+  JvmtiTagMap::weak_oops_do(is_alive, f);
+}
+
+void JvmtiExport::gc_epilogue() {
+  JvmtiCurrentBreakpoints::gc_epilogue();
+}
+
 // Onload raw monitor transition.
 void JvmtiExport::transition_pending_onload_raw_monitors() {
   JvmtiPendingMonitors::transition_raw_monitors();

src/share/vm/prims/jvmtiExport.hpp

@@ -346,6 +346,8 @@ class JvmtiExport : public AllStatic {
   static void cleanup_thread             (JavaThread* thread) KERNEL_RETURN;
 
   static void oops_do(OopClosure* f) KERNEL_RETURN;
+  static void weak_oops_do(BoolObjectClosure* b, OopClosure* f) KERNEL_RETURN;
+  static void gc_epilogue() KERNEL_RETURN;
 
   static void transition_pending_onload_raw_monitors() KERNEL_RETURN;
 

src/share/vm/prims/jvmtiImpl.cpp

@@ -212,7 +212,13 @@ void GrowableCache::oops_do(OopClosure* f) {
   for (int i=0; i<len; i++) {
     GrowableElement *e = _elements->at(i);
     e->oops_do(f);
-    _cache[i] = e->getCacheValue();
+  }
+}
+
+void GrowableCache::gc_epilogue() {
+  int len = _elements->length();
+  for (int i=0; i<len; i++) {
+    _cache[i] = _elements->at(i)->getCacheValue();
   }
 }
 
@@ -394,6 +400,10 @@ void  JvmtiBreakpoints::oops_do(OopClosure* f) {
   _bps.oops_do(f);
 }
 
+void JvmtiBreakpoints::gc_epilogue() {
+  _bps.gc_epilogue();
+}
+
 void  JvmtiBreakpoints::print() {
 #ifndef PRODUCT
   ResourceMark rm;
@@ -523,6 +533,12 @@ void JvmtiCurrentBreakpoints::oops_do(OopClosure* f) {
   }
 }
 
+void JvmtiCurrentBreakpoints::gc_epilogue() {
+  if (_jvmti_breakpoints != NULL) {
+    _jvmti_breakpoints->gc_epilogue();
+  }
+}
+
 ///////////////////////////////////////////////////////////////
 //
 // class VM_GetOrSetLocal

src/share/vm/prims/jvmtiImpl.hpp

@@ -117,6 +117,8 @@ public:
   void clear();
   // apply f to every element and update the cache
   void oops_do(OopClosure* f);
+  // update the cache after a full gc
+  void gc_epilogue();
 };
 
 
@@ -148,6 +150,7 @@ public:
   void remove (int index)               { _cache.remove(index); }
   void clear()                          { _cache.clear(); }
   void oops_do(OopClosure* f)           { _cache.oops_do(f); }
+  void gc_epilogue()                    { _cache.gc_epilogue(); }
 };
 
 
@@ -282,6 +285,7 @@ public:
   int  clear(JvmtiBreakpoint& bp);
   void clearall_in_class_at_safepoint(klassOop klass);
   void clearall();
+  void gc_epilogue();
 };
 
 
@@ -325,6 +329,7 @@ public:
   static inline bool is_breakpoint(address bcp);
 
   static void oops_do(OopClosure* f);
+  static void gc_epilogue();
 };
 
 // quickly test whether the bcp matches a cached breakpoint in the list

src/share/vm/runtime/jniHandles.cpp

@@ -25,7 +25,7 @@
 #include "precompiled.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "oops/oop.inline.hpp"
-#include "prims/jvmtiTagMap.hpp"
+#include "prims/jvmtiExport.hpp"
 #include "runtime/jniHandles.hpp"
 #include "runtime/mutexLocker.hpp"
 #ifdef TARGET_OS_FAMILY_linux
@@ -431,10 +431,10 @@ void JNIHandleBlock::weak_oops_do(BoolObjectClosure* is_alive,
   }
 
   /*
-   * JvmtiTagMap may also contain weak oops.  The iteration of it is placed
-   * here so that we don't need to add it to each of the collectors.
+   * JVMTI data structures may also contain weak oops.  The iteration of them
+   * is placed here so that we don't need to add it to each of the collectors.
    */
-  JvmtiTagMap::weak_oops_do(is_alive, f);
+  JvmtiExport::weak_oops_do(is_alive, f);
 }