8373203: Genshen: Non-strong reference leak in old gen

src/hotspot/share/gc/shenandoah/heuristics/shenandoahYoungHeuristics.hpp

@@ -44,6 +44,9 @@ class ShenandoahYoungHeuristics : public ShenandoahGenerationalHeuristics {
 
   bool should_start_gc() override;
 
+  // Young collections can never unload classes
+  bool can_unload_classes() override { return false; }
+
   size_t bytes_of_allocation_runway_before_gc_trigger(size_t young_regions_to_be_reclaimed);
 
 private:

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp

@@ -670,7 +670,7 @@ void ShenandoahConcurrentGC::op_init_mark() {
 
   if (heap->mode()->is_generational()) {
     if (_generation->is_global()) {
-      heap->old_generation()->cancel_gc();
+      heap->old_generation()->abandon_gc();
     }
 
     {
@@ -696,18 +696,17 @@ void ShenandoahConcurrentGC::op_init_mark() {
 
   start_mark();
 
-  if (_do_old_gc_bootstrap) {
-    shenandoah_assert_generational();
-    // Update region state for both young and old regions
-    ShenandoahGCPhase phase(ShenandoahPhaseTimings::init_update_region_states);
-    ShenandoahInitMarkUpdateRegionStateClosure cl;
-    heap->parallel_heap_region_iterate(&cl);
-    heap->old_generation()->ref_processor()->reset_thread_locals();
-  } else {
-    // Update region state for only young regions
+  {
     ShenandoahGCPhase phase(ShenandoahPhaseTimings::init_update_region_states);
     ShenandoahInitMarkUpdateRegionStateClosure cl;
-    _generation->parallel_heap_region_iterate(&cl);
+    if (_do_old_gc_bootstrap) {
+      // Update region state for both young and old regions
+      shenandoah_assert_generational();
+      heap->parallel_heap_region_iterate(&cl);
+    } else {
+      // Update region state for only current generation regions
+      _generation->parallel_heap_region_iterate(&cl);
+    }
   }
 
   // Weak reference processing
@@ -1098,7 +1097,7 @@ void ShenandoahConcurrentGC::op_init_update_refs() {
   }
 }
 
-void ShenandoahConcurrentGC::op_update_refs() {
+void ShenandoahConcurrentGC::op_update_refs() const {
   ShenandoahHeap::heap()->update_heap_references(_generation, true /*concurrent*/);
 }
 

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp

@@ -118,7 +118,7 @@ class ShenandoahConcurrentGC : public ShenandoahGC {
   void op_cleanup_early();
   void op_evacuate();
   void op_init_update_refs();
-  void op_update_refs();
+  void op_update_refs() const;
   void op_update_thread_roots();
   void op_final_update_refs();
 

src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp

@@ -162,10 +162,11 @@ void ShenandoahDegenGC::op_degenerated() {
       } else {
         if (_generation->is_concurrent_mark_in_progress()) {
           // We want to allow old generation marking to be punctuated by young collections
-          // (even if they have degenerated). If this is a global cycle, we'd have cancelled
+          // (even if they have degenerated). If this is a global cycle, we'd have abandoned
           // the entire old gc before coming into this switch. Note that cancel_marking on
-          // the generation does NOT abandon incomplete SATB buffers as cancel_concurrent_mark does.
-          // We need to separate out the old pointers which is done below.
+          // the young generation does NOT abandon incomplete SATB buffers in the old generation
+          // as cancel_concurrent_mark does. We need to separate out the old pointers which
+          // is done below.
           _generation->cancel_marking();
         }
 

src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp

@@ -104,7 +104,7 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
 
   virtual ShenandoahHeuristics* heuristics() const { return _heuristics; }
 
-  ShenandoahReferenceProcessor* ref_processor() { return _ref_processor; }
+  ShenandoahReferenceProcessor* ref_processor() const { return _ref_processor; }
 
   virtual ShenandoahHeuristics* initialize_heuristics(ShenandoahMode* gc_mode);
 

src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp

@@ -170,19 +170,18 @@ ShenandoahGenerationalControlThread::GCMode ShenandoahGenerationalControlThread:
 }
 
 ShenandoahGenerationalControlThread::GCMode ShenandoahGenerationalControlThread::prepare_for_explicit_gc(ShenandoahGCRequest &request) const {
-  ShenandoahHeuristics* global_heuristics = _heap->global_generation()->heuristics();
-  request.generation = _heap->global_generation();
-  global_heuristics->log_trigger("GC request (%s)", GCCause::to_string(request.cause));
-  global_heuristics->record_requested_gc();
+  ShenandoahHeuristics* heuristics = request.generation->heuristics();
+  heuristics->log_trigger("GC request (%s)", GCCause::to_string(request.cause));
+  heuristics->record_requested_gc();
 
   if (ShenandoahCollectorPolicy::should_run_full_gc(request.cause)) {
-    return stw_full;;
-  } else {
-    // Unload and clean up everything. Note that this is an _explicit_ request and so does not use
-    // the same `should_unload_classes` call as the regulator's concurrent gc request.
-    _heap->set_unload_classes(global_heuristics->can_unload_classes());
-    return concurrent_normal;
+    return stw_full;
   }
+
+  // Unload and clean up everything. Note that this is an _explicit_ request and so does not use
+  // the same `should_unload_classes` call as the regulator's concurrent gc request.
+  _heap->set_unload_classes(heuristics->can_unload_classes());
+  return concurrent_normal;
 }
 
 ShenandoahGenerationalControlThread::GCMode ShenandoahGenerationalControlThread::prepare_for_concurrent_gc(const ShenandoahGCRequest &request) const {
@@ -407,15 +406,11 @@ void ShenandoahGenerationalControlThread::service_concurrent_old_cycle(const She
       return;
     }
     case ShenandoahOldGeneration::WAITING_FOR_BOOTSTRAP:
+      // Configure the young generation for bootstrapping the old mark
+      young_generation->prepare_for_bootstrap(old_generation);
       old_generation->transition_to(ShenandoahOldGeneration::BOOTSTRAPPING);
     case ShenandoahOldGeneration::BOOTSTRAPPING: {
-      // Configure the young generation's concurrent mark to put objects in
-      // old regions into the concurrent mark queues associated with the old
-      // generation. The young cycle will run as normal except that rather than
-      // ignore old references it will mark and enqueue them in the old concurrent
-      // task queues but it will not traverse them.
       set_gc_mode(bootstrapping_old);
-      young_generation->set_old_gen_task_queues(old_generation->task_queues());
       service_concurrent_cycle(young_generation, request.cause, true);
       _heap->process_gc_stats();
       if (_heap->cancelled_gc()) {
@@ -561,10 +556,10 @@ void ShenandoahGenerationalControlThread::service_concurrent_cycle(ShenandoahGen
       }
     }
   } else {
-    assert(generation->is_global(), "If not young, must be GLOBAL");
-    assert(!do_old_gc_bootstrap, "Do not bootstrap with GLOBAL GC");
+    assert(generation->is_global(), "If not young, must be Global");
+    assert(!do_old_gc_bootstrap, "Do not bootstrap with Global GC");
     if (_heap->cancelled_gc()) {
-      msg = "At end of Interrupted Concurrent GLOBAL GC";
+      msg = "At end of Interrupted Concurrent Global GC";
     } else {
       // We only record GC results if GC was successful
       msg = "At end of Concurrent Global GC";
@@ -715,16 +710,7 @@ bool ShenandoahGenerationalControlThread::preempt_old_marking(ShenandoahGenerati
   return generation->is_young() && _allow_old_preemption.try_unset();
 }
 
-void ShenandoahGenerationalControlThread::handle_requested_gc(GCCause::Cause cause) {
-  // For normal requested GCs (System.gc) we want to block the caller. However,
-  // for whitebox requested GC, we want to initiate the GC and return immediately.
-  // The whitebox caller thread will arrange for itself to wait until the GC notifies
-  // it that has reached the requested breakpoint (phase in the GC).
-  if (cause == GCCause::_wb_breakpoint) {
-    notify_control_thread(cause, ShenandoahHeap::heap()->global_generation());
-    return;
-  }
-
+void ShenandoahGenerationalControlThread::wait_for_gc_cycle(GCCause::Cause cause, ShenandoahGeneration* generation) {
   // Make sure we have at least one complete GC cycle before unblocking
   // from the explicit GC request.
   //
@@ -739,12 +725,27 @@ void ShenandoahGenerationalControlThread::handle_requested_gc(GCCause::Cause cau
   const size_t required_gc_id = current_gc_id + 1;
   while (current_gc_id < required_gc_id && !should_terminate()) {
     // Make requests to run a global cycle until at least one is completed
-    notify_control_thread(cause, ShenandoahHeap::heap()->global_generation());
+    notify_control_thread(cause, generation);
     ml.wait();
     current_gc_id = get_gc_id();
   }
 }
 
+void ShenandoahGenerationalControlThread::handle_requested_gc(GCCause::Cause cause) {
+  // For normal requested GCs (System.gc) we want to block the caller. However,
+  // for whitebox requested GC, we want to initiate the GC and return immediately.
+  // The whitebox caller thread will arrange for itself to wait until the GC notifies
+  // it that has reached the requested breakpoint (phase in the GC).
+  if (cause == GCCause::_wb_breakpoint) {
+    notify_control_thread(cause, ShenandoahHeap::heap()->global_generation());
+    return;
+  }
+  ShenandoahGeneration* generation = cause == GCCause::_wb_young_gc
+                                   ? ShenandoahHeap::heap()->young_generation()
+                                   : ShenandoahHeap::heap()->global_generation();
+  wait_for_gc_cycle(cause, generation);
+}
+
 void ShenandoahGenerationalControlThread::notify_gc_waiters() {
   MonitorLocker ml(&_gc_waiters_lock);
   ml.notify_all();
@@ -770,8 +771,9 @@ void ShenandoahGenerationalControlThread::set_gc_mode(GCMode new_mode) {
 
 void ShenandoahGenerationalControlThread::set_gc_mode(MonitorLocker& ml, GCMode new_mode) {
   if (_gc_mode != new_mode) {
-    log_debug(gc, thread)("Transition from: %s to: %s", gc_mode_name(_gc_mode), gc_mode_name(new_mode));
-    EventMark event("Control thread transition from: %s, to %s", gc_mode_name(_gc_mode), gc_mode_name(new_mode));
+    FormatBuffer<> msg("Transition from: %s to: %s", gc_mode_name(_gc_mode), gc_mode_name(new_mode));
+    log_debug(gc, thread)("%s", msg.buffer());
+    Events::log(this, "%s", msg.buffer());
     _gc_mode = new_mode;
     ml.notify_all();
   }

src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp

@@ -98,6 +98,9 @@ class ShenandoahGenerationalControlThread: public ShenandoahController {
   // Return true if the request to start a concurrent GC for the given generation succeeded.
   bool request_concurrent_gc(ShenandoahGeneration* generation);
 
+  // Visible for white box API to start an old cycle
+  void wait_for_gc_cycle(GCCause::Cause cause, ShenandoahGeneration* generation);
+
   // Returns the current state of the control thread
   GCMode gc_mode() const {
     return _gc_mode;

src/hotspot/share/gc/shenandoah/shenandoahGenerationalFullGC.cpp

@@ -58,8 +58,11 @@ void ShenandoahGenerationalFullGC::prepare() {
   // No need for old_gen->increase_used() as this was done when plabs were allocated.
   heap->reset_generation_reserves();
 
+  // If we were bootstrapping, we don't need that configuration anymore
+  heap->young_generation()->clear_bootstrap_configuration();
+
   // Full GC supersedes any marking or coalescing in old generation.
-  heap->old_generation()->cancel_gc();
+  heap->old_generation()->abandon_gc();
 }
 
 void ShenandoahGenerationalFullGC::handle_completion(ShenandoahHeap* heap) {

src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.cpp

@@ -127,6 +127,11 @@ void ShenandoahGenerationalHeap::post_initialize_heuristics() {
   _old_generation->post_initialize(this);
 }
 
+bool ShenandoahGenerationalHeap::start_old_collection() {
+  static_cast<ShenandoahGenerationalControlThread*>(_control_thread)->wait_for_gc_cycle(GCCause::_shenandoah_concurrent_gc, old_generation());
+  return true;
+}
+
 void ShenandoahGenerationalHeap::initialize_serviceability() {
   assert(mode()->is_generational(), "Only for the generational mode");
   _young_gen_memory_pool = new ShenandoahYoungGenMemoryPool(this);
@@ -938,6 +943,17 @@ class ShenandoahGenerationalUpdateHeapRefsTask : public WorkerTask {
 
 void ShenandoahGenerationalHeap::update_heap_references(ShenandoahGeneration* generation, bool concurrent) {
   assert(!is_full_gc_in_progress(), "Only for concurrent and degenerated GC");
+
+
+  ShenandoahReferenceProcessor* old_ref_processor = generation->ref_processor()->get_old_generation_ref_processor();
+  if (old_ref_processor != nullptr) {
+    // Discovered lists may have young references with old referents. These references will be
+    // processed at the end of old marking. We need to update them.
+    assert(generation->is_young(), "We should only have old discovered lists in a young collection");
+    ShenandoahPhaseTimings::Phase phase = concurrent ? ShenandoahPhaseTimings::conc_weak_refs : ShenandoahPhaseTimings::degen_gc_weakrefs;
+    old_ref_processor->heal_discovered_lists(phase, workers(), concurrent);
+  }
+
   const uint nworkers = workers()->active_workers();
   ShenandoahRegionChunkIterator work_list(nworkers);
   if (concurrent) {
@@ -1026,9 +1042,8 @@ void ShenandoahGenerationalHeap::final_update_refs_update_region_states() {
 
 void ShenandoahGenerationalHeap::complete_degenerated_cycle() {
   shenandoah_assert_heaplocked_or_safepoint();
-  // In case degeneration interrupted concurrent evacuation or update references, we need to clean up
-  // transient state. Otherwise, these actions have no effect.
-  reset_generation_reserves();
+
+  complete_cycle();
 
   if (!old_generation()->is_parsable()) {
     ShenandoahGCPhase phase(ShenandoahPhaseTimings::degen_gc_coalesce_and_fill);
@@ -1037,6 +1052,8 @@ void ShenandoahGenerationalHeap::complete_degenerated_cycle() {
 }
 
 void ShenandoahGenerationalHeap::complete_concurrent_cycle() {
+  complete_cycle();
+
   if (!old_generation()->is_parsable()) {
     // Class unloading may render the card offsets unusable, so we must rebuild them before
     // the next remembered set scan. We _could_ let the control thread do this sometime after
@@ -1047,6 +1064,17 @@ void ShenandoahGenerationalHeap::complete_concurrent_cycle() {
     // throw off the heuristics.
     entry_global_coalesce_and_fill();
   }
+
+}
+
+void ShenandoahGenerationalHeap::complete_cycle() {
+  if (young_generation()->is_bootstrap_cycle()) {
+    // Once the bootstrap cycle is completed, the young generation is no longer obliged to mark old
+    young_generation()->clear_bootstrap_configuration();
+  }
+
+  // In case degeneration interrupted concurrent evacuation or update references, we need to clean up
+  // transient state. Otherwise, these actions have no effect.
   reset_generation_reserves();
 }
 

src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp

@@ -43,6 +43,8 @@ class ShenandoahGenerationalHeap : public ShenandoahHeap {
   void initialize_heuristics() override;
   void post_initialize_heuristics() override;
 
+  bool start_old_collection();
+
   static ShenandoahGenerationalHeap* heap() {
     assert(ShenandoahCardBarrier, "Should have card barrier to use genenrational heap");
     CollectedHeap* heap = Universe::heap();
@@ -142,6 +144,7 @@ class ShenandoahGenerationalHeap : public ShenandoahHeap {
   void complete_degenerated_cycle();
   void complete_concurrent_cycle();
 private:
+  void complete_cycle();
   void initialize_controller() override;
   void entry_global_coalesce_and_fill();
 

src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp

@@ -57,10 +57,6 @@ void ShenandoahOldGC::op_final_mark() {
     _mark.finish_mark();
     assert(!heap->cancelled_gc(), "STW mark cannot OOM");
 
-    // Old collection is complete, the young generation no longer needs this
-    // reference to the old concurrent mark so clean it up.
-    heap->young_generation()->set_old_gen_task_queues(nullptr);
-
     // We need to do this because weak root cleaning reports the number of dead handles
     JvmtiTagMap::set_needs_cleaning();
 

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp

@@ -293,7 +293,7 @@ void ShenandoahOldGeneration::cancel_marking() {
   ShenandoahGeneration::cancel_marking();
 }
 
-void ShenandoahOldGeneration::cancel_gc() {
+void ShenandoahOldGeneration::abandon_gc() {
   shenandoah_assert_safepoint();
   if (is_idle()) {
 #ifdef ASSERT
@@ -305,8 +305,6 @@ void ShenandoahOldGeneration::cancel_gc() {
     cancel_marking();
     // Stop tracking old regions
     abandon_collection_candidates();
-    // Remove old generation access to young generation mark queues
-    ShenandoahHeap::heap()->young_generation()->set_old_gen_task_queues(nullptr);
     // Transition to IDLE now.
     transition_to(ShenandoahOldGeneration::WAITING_FOR_BOOTSTRAP);
   }
@@ -446,8 +444,9 @@ const char* ShenandoahOldGeneration::state_name(State state) {
 
 void ShenandoahOldGeneration::transition_to(State new_state) {
   if (_state != new_state) {
-    log_debug(gc, thread)("Old generation transition from %s to %s", state_name(_state), state_name(new_state));
-    EventMark event("Old was %s, now is %s", state_name(_state), state_name(new_state));
+    FormatBuffer<> msg("Old was %s, now is %s", state_name(_state), state_name(new_state));
+    log_debug(gc, thread)("%s", msg.buffer());
+    Events::log(Thread::current(), "%s", msg.buffer());
     validate_transition(new_state);
     _state = new_state;
   }
@@ -531,10 +530,11 @@ void ShenandoahOldGeneration::validate_transition(State new_state) {
       assert(_state == WAITING_FOR_BOOTSTRAP, "Cannot reset bitmap without making old regions parsable, state is '%s'", state_name(_state));
       assert(_old_heuristics->unprocessed_old_collection_candidates() == 0, "Cannot bootstrap with mixed collection candidates");
       assert(!heap->is_prepare_for_old_mark_in_progress(), "Cannot still be making old regions parsable.");
+      assert(heap->young_generation()->is_bootstrap_cycle(), "Young generation needs old mark queues.");
       break;
     case MARKING:
       assert(_state == BOOTSTRAPPING, "Must have finished bootstrapping before marking, state is '%s'", state_name(_state));
-      assert(heap->young_generation()->old_gen_task_queues() != nullptr, "Young generation needs old mark queues.");
+      assert(!heap->young_generation()->is_bootstrap_cycle(), "Young generation is done with bootstrapping");
       assert(heap->is_concurrent_old_mark_in_progress(), "Should be marking old now.");
       break;
     case EVACUATING_AFTER_GLOBAL:
@@ -552,7 +552,6 @@ void ShenandoahOldGeneration::validate_transition(State new_state) {
 bool ShenandoahOldGeneration::validate_waiting_for_bootstrap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   assert(!heap->is_concurrent_old_mark_in_progress(), "Cannot become ready for bootstrap during old mark.");
-  assert(heap->young_generation()->old_gen_task_queues() == nullptr, "Cannot become ready for bootstrap when still setup for bootstrapping.");
   assert(!is_concurrent_mark_in_progress(), "Cannot be marking in IDLE");
   assert(!heap->young_generation()->is_bootstrap_cycle(), "Cannot have old mark queues if IDLE");
   assert(!_old_heuristics->has_coalesce_and_fill_candidates(), "Cannot have coalesce and fill candidates in IDLE");

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp

@@ -225,8 +225,8 @@ class ShenandoahOldGeneration : public ShenandoahGeneration {
   void record_success_concurrent(bool abbreviated) override;
   void cancel_marking() override;
 
-  // Cancels old gc and transitions to the idle state
-  void cancel_gc();
+  // Abandons all old gc state and transitions to the idle state
+  void abandon_gc();
 
   // The SATB barrier will be "enabled" until old marking completes. This means it is
   // possible for an entire young collection cycle to execute while the SATB barrier is enabled.