8300915: G1: incomplete SATB because nmethod entry barriers don't get armed

Reviewed-by: tschatzl, eosterlund

src/hotspot/share/code/codeCache.cpp

@@ -863,6 +863,9 @@ void CodeCache::on_gc_marking_cycle_start() {
   ++_gc_epoch;
 }
 
+// Once started the code cache marking cycle must only be finished after marking of
+// the java heap is complete. Otherwise nmethods could appear to be not on stack even
+// if they have frames in continuation StackChunks that were not yet visited.
 void CodeCache::on_gc_marking_cycle_finish() {
   assert(is_gc_marking_cycle_active(), "Marking cycle started before last one finished");
   ++_gc_epoch;

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -3360,15 +3360,23 @@ void G1CollectedHeap::fill_with_dummy_object(HeapWord* start, HeapWord* end, boo
   region->fill_with_dummy_object(start, pointer_delta(end, start), zap);
 }
 
-void G1CollectedHeap::start_codecache_marking_cycle_if_inactive() {
+void G1CollectedHeap::start_codecache_marking_cycle_if_inactive(bool concurrent_mark_start) {
+  // We can reach here with an active code cache marking cycle either because the
+  // previous G1 concurrent marking cycle was undone (if heap occupancy after the
+  // concurrent start young collection was below the threshold) or aborted. See
+  // CodeCache::on_gc_marking_cycle_finish() why this is.  We must not start a new code
+  // cache cycle then. If we are about to start a new g1 concurrent marking cycle we
+  // still have to arm all nmethod entry barriers. They are needed for adding oop
+  // constants to the SATB snapshot. Full GC does not need nmethods to be armed.
   if (!CodeCache::is_gc_marking_cycle_active()) {
-    // This is the normal case when we do not call collect when a
-    // concurrent mark is ongoing. We then start a new code marking
-    // cycle. If, on the other hand, a concurrent mark is ongoing, we
-    // will be conservative and use the last code marking cycle. Code
-    // caches marked between the two concurrent marks will live a bit
-    // longer than needed.
     CodeCache::on_gc_marking_cycle_start();
+  }
+  if (concurrent_mark_start) {
     CodeCache::arm_all_nmethods();
   }
 }
+
+void G1CollectedHeap::finish_codecache_marking_cycle() {
+  CodeCache::on_gc_marking_cycle_finish();
+  CodeCache::arm_all_nmethods();
+}

src/hotspot/share/gc/g1/g1CollectedHeap.hpp

@@ -933,7 +933,8 @@ public:
 
   void fill_with_dummy_object(HeapWord* start, HeapWord* end, bool zap) override;
 
-  static void start_codecache_marking_cycle_if_inactive();
+  static void start_codecache_marking_cycle_if_inactive(bool concurrent_mark_start);
+  static void finish_codecache_marking_cycle();
 
   // Apply the given closure on all cards in the Hot Card Cache, emptying it.
   void iterate_hcc_closure(G1CardTableEntryClosure* cl, uint worker_id);

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

@@ -792,7 +792,7 @@ G1PreConcurrentStartTask::G1PreConcurrentStartTask(GCCause::Cause cause, G1Concu
 void G1ConcurrentMark::pre_concurrent_start(GCCause::Cause cause) {
   assert_at_safepoint_on_vm_thread();
 
-  G1CollectedHeap::start_codecache_marking_cycle_if_inactive();
+  G1CollectedHeap::start_codecache_marking_cycle_if_inactive(true /* concurrent_mark_start */);
 
   ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_strong);
 
@@ -1300,6 +1300,8 @@ void G1ConcurrentMark::remark() {
     assert(!restart_for_overflow(), "sanity");
     // Completely reset the marking state (except bitmaps) since marking completed.
     reset_at_marking_complete();
+
+    G1CollectedHeap::finish_codecache_marking_cycle();
   } else {
     // We overflowed.  Restart concurrent marking.
     _restart_for_overflow = true;
@@ -1316,9 +1318,6 @@ void G1ConcurrentMark::remark() {
     report_object_count(mark_finished);
   }
 
-  CodeCache::on_gc_marking_cycle_finish();
-  CodeCache::arm_all_nmethods();
-
   // Statistics
   double now = os::elapsedTime();
   _remark_mark_times.add((mark_work_end - start) * 1000.0);

src/hotspot/share/gc/g1/g1FullCollector.cpp

@@ -197,7 +197,7 @@ void G1FullCollector::prepare_collection() {
 }
 
 void G1FullCollector::collect() {
-  G1CollectedHeap::start_codecache_marking_cycle_if_inactive();
+  G1CollectedHeap::start_codecache_marking_cycle_if_inactive(false /* concurrent_mark_start */);
 
   phase1_mark_live_objects();
   verify_after_marking();
@@ -211,8 +211,7 @@ void G1FullCollector::collect() {
 
   phase4_do_compaction();
 
-  CodeCache::on_gc_marking_cycle_finish();
-  CodeCache::arm_all_nmethods();
+  G1CollectedHeap::finish_codecache_marking_cycle();
 }
 
 void G1FullCollector::complete_collection() {