8328235: GenShen: Robustify ShenandoahGCSession and fix missing use

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

@@ -173,6 +173,17 @@ class ShenandoahAsserts {
 
 #define shenandoah_assert_generational() \
                     assert(ShenandoahHeap::heap()->mode()->is_generational(), "Must be in generational mode here.")
+
+// Some limited sanity checking of the _gc_generation and _active_generation fields of ShenandoahHeap
+#define shenandoah_assert_generations_reconciled()                                                             \
+                    if (SafepointSynchronize::is_at_safepoint()) {                                             \
+                      ShenandoahHeap* heap = ShenandoahHeap::heap();                                           \
+                      ShenandoahGeneration* ggen = heap->gc_generation();                                      \
+                      ShenandoahGeneration* agen = heap->active_generation();                                  \
+                      assert(agen == ggen, "active_gen(%d) should be reconciled with gc_gen(%d)at safepoint",  \
+                             agen->type(), ggen->type());                                                      \
+                    }
+
 #else
 #define shenandoah_assert_in_heap(interior_loc, obj)
 #define shenandoah_assert_in_heap_or_null(interior_loc, obj)
@@ -226,6 +237,7 @@ class ShenandoahAsserts {
 #define shenandoah_assert_heaplocked_or_fullgc_safepoint()
 #define shenandoah_assert_control_or_vm_thread()
 #define shenandoah_assert_generational()
+#define shenandoah_assert_generations_reconciled()                                                             \
 
 #endif
 

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

@@ -928,7 +928,7 @@ void ShenandoahEvacUpdateCleanupOopStorageRootsClosure::do_oop(oop* p) {
   const oop obj = RawAccess<>::oop_load(p);
   if (!CompressedOops::is_null(obj)) {
     if (!_mark_context->is_marked(obj)) {
-      _heap->assert_generations_reconciled();
+      shenandoah_assert_generations_reconciled();
       if (_heap->is_in_active_generation(obj)) {
         // TODO: This worries me. Here we are asserting that an unmarked from-space object is 'correct'.
         // Normally, I would call this a bogus assert, but there seems to be a legitimate use-case for

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

@@ -66,7 +66,7 @@ class ShenandoahConcurrentMarkingTask : public WorkerTask {
     // Do not use active_generation() : we must use the gc_generation() set by
     // ShenandoahGCScope on the ControllerThread's stack; no safepoint may
     // intervene to update active_generation, so we can't
-    // assert_generations_reconciled() here.
+    // shenandoah_assert_generations_reconciled() here.
     ShenandoahReferenceProcessor* rp = heap->gc_generation()->ref_processor();
     assert(rp != nullptr, "need reference processor");
     StringDedup::Requests requests;
@@ -116,7 +116,7 @@ class ShenandoahFinalMarkingTask : public WorkerTask {
     ShenandoahParallelWorkerSession worker_session(worker_id);
     StringDedup::Requests requests;
     ShenandoahReferenceProcessor* rp = heap->gc_generation()->ref_processor();
-    heap->assert_generations_reconciled();
+    shenandoah_assert_generations_reconciled();
 
     // First drain remaining SATB buffers.
     {

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

@@ -132,7 +132,7 @@ void ShenandoahGenerationalEvacuationTask::promote_in_place(ShenandoahHeapRegion
   {
     const size_t old_garbage_threshold = (ShenandoahHeapRegion::region_size_bytes() * ShenandoahOldGarbageThreshold) / 100;
     assert(_heap->gc_generation()->is_mark_complete(), "sanity");
-    _heap->assert_generations_reconciled();
+    shenandoah_assert_generations_reconciled();
     assert(!_heap->is_concurrent_old_mark_in_progress(), "Cannot promote in place during old marking");
     assert(region->garbage_before_padded_for_promote() < old_garbage_threshold, "Region " SIZE_FORMAT " has too much garbage for promotion", region->index());
     assert(region->is_young(), "Only young regions can be promoted");
@@ -216,7 +216,7 @@ void ShenandoahGenerationalEvacuationTask::promote_humongous(ShenandoahHeapRegio
   ShenandoahMarkingContext* marking_context = _heap->marking_context();
   oop obj = cast_to_oop(region->bottom());
   assert(_heap->gc_generation()->is_mark_complete(), "sanity");
-  _heap->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   assert(region->is_young(), "Only young regions can be promoted");
   assert(region->is_humongous_start(), "Should not promote humongous continuation in isolation");
   assert(region->age() >= _heap->age_census()->tenuring_threshold(), "Only promote regions that are sufficiently aged");

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

@@ -760,7 +760,7 @@ class ShenandoahGenerationalUpdateHeapRefsTask : public WorkerTask {
     ShenandoahHeapRegion* r = _regions->next();
     // We update references for global, old, and young collections.
     ShenandoahGeneration* const gc_generation = _heap->gc_generation();
-    _heap->assert_generations_reconciled();
+    shenandoah_assert_generations_reconciled();
     assert(gc_generation->is_mark_complete(), "Expected complete marking");
     ShenandoahMarkingContext* const ctx = _heap->marking_context();
     bool is_mixed = _heap->collection_set()->has_old_regions();

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

@@ -1587,17 +1587,6 @@ void ShenandoahHeap::set_active_generation() {
   _active_generation = _gc_generation;
 }
 
-#ifndef PRODUCT
-void ShenandoahHeap::assert_generations_reconciled() {
-  if (SafepointSynchronize::is_at_safepoint()) {
-    ShenandoahGeneration* ggen = gc_generation();
-    ShenandoahGeneration* agen = active_generation();
-    assert(agen == ggen, "active_gen(%d) should be reconciled with gc_gen(%d)at safepoint",
-           agen->type(), ggen->type());
-  }
-}
-#endif
-
 void ShenandoahHeap::on_cycle_start(GCCause::Cause cause, ShenandoahGeneration* generation) {
   shenandoah_policy()->record_collection_cause(cause);
 
@@ -1974,7 +1963,7 @@ void ShenandoahHeap::stw_weak_refs(bool full_gc) {
                                                 : ShenandoahPhaseTimings::degen_gc_weakrefs;
   ShenandoahTimingsTracker t(phase);
   ShenandoahGCWorkerPhase worker_phase(phase);
-  assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   gc_generation()->ref_processor()->process_references(phase, workers(), false /* concurrent */);
 }
 
@@ -2009,7 +1998,7 @@ void ShenandoahHeap::set_gc_state(uint mask, bool value) {
   _gc_state_changed = true;
   // Check that if concurrent weak root is set then active_gen isn't null
   assert(!is_concurrent_weak_root_in_progress() || active_generation() != nullptr, "Error");
-  assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
 }
 
 void ShenandoahHeap::set_concurrent_young_mark_in_progress(bool in_progress) {
@@ -2303,7 +2292,7 @@ void ShenandoahHeap::sync_pinned_region_status() {
 void ShenandoahHeap::assert_pinned_region_status() {
   for (size_t i = 0; i < num_regions(); i++) {
     ShenandoahHeapRegion* r = get_region(i);
-    assert_generations_reconciled();
+    shenandoah_assert_generations_reconciled();
     if (gc_generation()->contains(r)) {
       assert((r->is_pinned() && r->pin_count() > 0) || (!r->is_pinned() && r->pin_count() == 0),
              "Region " SIZE_FORMAT " pinning status is inconsistent", i);

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

@@ -187,10 +187,6 @@ class ShenandoahHeap : public CollectedHeap {
   // a safepoint by the VMThread.
   void set_active_generation();
 
-  // Some limited sanity checking of the _gc_generation and
-  // _active_generation fields
-  void assert_generations_reconciled() PRODUCT_RETURN;
-
   ShenandoahHeuristics* heuristics();
 
 // ---------- Initialization, termination, identification, printing routines

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

@@ -319,7 +319,7 @@ void ShenandoahHeapRegion::make_trash_immediate() {
   // On this path, we know there are no marked objects in the region,
   // tell marking context about it to bypass bitmap resets.
   assert(ShenandoahHeap::heap()->gc_generation()->is_mark_complete(), "Marking should be complete here.");
-  ShenandoahHeap::heap()->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   ShenandoahHeap::heap()->marking_context()->reset_top_bitmap(this);
 }
 
@@ -470,7 +470,7 @@ bool ShenandoahHeapRegion::oop_coalesce_and_fill(bool cancellable) {
 
   // Expect marking to be completed before these threads invoke this service.
   assert(heap->gc_generation()->is_mark_complete(), "sanity");
-  heap->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
 
   // All objects above TAMS are considered live even though their mark bits will not be set.  Note that young-
   // gen evacuations that interrupt a long-running old-gen concurrent mark may promote objects into old-gen

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

@@ -152,7 +152,7 @@ void ShenandoahMark::mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint w
   // Do not use active_generation() : we must use the gc_generation() set by
   // ShenandoahGCScope on the ControllerThread's stack; no safepoint may
   // intervene to update active_generation, so we can't
-  // assert_generations_reconciled() here.
+  // shenandoah_assert_generations_reconciled() here.
   assert(heap->gc_generation()->type() == GENERATION, "Sanity: %d != %d", heap->gc_generation()->type(), GENERATION);
   heap->gc_generation()->ref_processor()->set_mark_closure(worker_id, cl);
 

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

@@ -105,7 +105,7 @@ void ShenandoahSTWMark::mark() {
 
   // Weak reference processing
   ShenandoahReferenceProcessor* rp = heap->gc_generation()->ref_processor();
-  heap->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   assert(heap->gc_generation() == heap->active_generation(), "Should be identical at stw phases");
   rp->reset_thread_locals();
   rp->set_soft_reference_policy(heap->soft_ref_policy()->should_clear_all_soft_refs());
@@ -175,7 +175,7 @@ void ShenandoahSTWMark::finish_mark(uint worker_id) {
   ShenandoahPhaseTimings::Phase phase = _full_gc ? ShenandoahPhaseTimings::full_gc_mark : ShenandoahPhaseTimings::degen_gc_stw_mark;
   ShenandoahWorkerTimingsTracker timer(phase, ShenandoahPhaseTimings::ParallelMark, worker_id);
   ShenandoahReferenceProcessor* rp = ShenandoahHeap::heap()->gc_generation()->ref_processor();
-  ShenandoahHeap::heap()->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   StringDedup::Requests requests;
 
   mark_loop(worker_id, &_terminator, rp,

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

@@ -96,7 +96,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
     if (_heap->mode()->is_generational()) {
       _generation = _heap->gc_generation();
       assert(_generation != nullptr, "Expected active generation in this mode");
-      _heap->assert_generations_reconciled();
+      shenandoah_assert_generations_reconciled();
     }
   }
 
@@ -192,7 +192,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
           check(ShenandoahAsserts::_safe_oop, obj, obj_reg->has_live() ||
                 (obj_reg->is_old() && _heap->gc_generation()->is_young()),
                    "Object must belong to region with live data");
-          _heap->assert_generations_reconciled();
+          shenandoah_assert_generations_reconciled();
           break;
         default:
           assert(false, "Unhandled liveness verification");
@@ -638,7 +638,7 @@ class ShenandoahVerifierMarkedRegionTask : public WorkerTask {
     if (_heap->mode()->is_generational()) {
       _generation = _heap->gc_generation();
       assert(_generation != nullptr, "Expected active generation in this mode.");
-      _heap->assert_generations_reconciled();
+      shenandoah_assert_generations_reconciled();
     }
   };
 
@@ -880,7 +880,7 @@ void ShenandoahVerifier::verify_at_safepoint(const char* label,
   if (_heap->mode()->is_generational()) {
     generation = _heap->gc_generation();
     guarantee(generation != nullptr, "Need to know which generation to verify.");
-    _heap->assert_generations_reconciled();
+    shenandoah_assert_generations_reconciled();
   } else {
     generation = nullptr;
   }
@@ -1353,7 +1353,7 @@ void ShenandoahVerifier::verify_rem_set_before_mark() {
   ShenandoahOldGeneration* old_generation = _heap->old_generation();
   log_debug(gc)("Verifying remembered set at %s mark", old_generation->is_doing_mixed_evacuations() ? "mixed" : "young");
 
-  _heap->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   if (old_generation->is_mark_complete() || _heap->gc_generation()->is_global()) {
     ctx = _heap->complete_marking_context();
   } else {
@@ -1434,7 +1434,7 @@ void ShenandoahVerifier::verify_rem_set_before_update_ref() {
 
   ShenandoahMarkingContext* ctx;
 
-  _heap->assert_generations_reconciled();
+  shenandoah_assert_generations_reconciled();
   if (_heap->old_generation()->is_mark_complete() || _heap->gc_generation()->is_global()) {
     ctx = _heap->complete_marking_context();
   } else {