8349094: GenShen: Race between control and regulator threads may violate assertions

Reviewed-by: ysr, kdnilsen

Author: William Kemper

src/hotspot/share/gc/shared/gcCause.hpp

@@ -74,6 +74,7 @@ class GCCause : public AllStatic {
 
     _shenandoah_stop_vm,
     _shenandoah_allocation_failure_evac,
+    _shenandoah_humongous_allocation_failure,
     _shenandoah_concurrent_gc,
     _shenandoah_upgrade_to_full_gc,
 

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp

@@ -60,7 +60,6 @@ int ShenandoahOldHeuristics::compare_by_index(RegionData a, RegionData b) {
 ShenandoahOldHeuristics::ShenandoahOldHeuristics(ShenandoahOldGeneration* generation, ShenandoahGenerationalHeap* gen_heap) :
   ShenandoahHeuristics(generation),
   _heap(gen_heap),
-  _old_gen(generation),
   _first_pinned_candidate(NOT_FOUND),
   _last_old_collection_candidate(0),
   _next_old_collection_candidate(0),
@@ -567,9 +566,9 @@ void ShenandoahOldHeuristics::set_trigger_if_old_is_fragmented(size_t first_old_
     // allocation request will require a STW full GC.
     size_t allowed_old_gen_span = num_regions - (ShenandoahGenerationalHumongousReserve * num_regions) / 100;
 
-    size_t old_available = _old_gen->available() / HeapWordSize;
+    size_t old_available = _old_generation->available() / HeapWordSize;
     size_t region_size_words = ShenandoahHeapRegion::region_size_words();
-    size_t old_unaffiliated_available = _old_gen->free_unaffiliated_regions() * region_size_words;
+    size_t old_unaffiliated_available = _old_generation->free_unaffiliated_regions() * region_size_words;
     assert(old_available >= old_unaffiliated_available, "sanity");
     size_t old_fragmented_available = old_available - old_unaffiliated_available;
 
@@ -603,12 +602,12 @@ void ShenandoahOldHeuristics::set_trigger_if_old_is_fragmented(size_t first_old_
 }
 
 void ShenandoahOldHeuristics::set_trigger_if_old_is_overgrown() {
-  size_t old_used = _old_gen->used() + _old_gen->get_humongous_waste();
-  size_t trigger_threshold = _old_gen->usage_trigger_threshold();
+  size_t old_used = _old_generation->used() + _old_generation->get_humongous_waste();
+  size_t trigger_threshold = _old_generation->usage_trigger_threshold();
   // Detects unsigned arithmetic underflow
   assert(old_used <= _heap->capacity(),
          "Old used (%zu, %zu) must not be more than heap capacity (%zu)",
-         _old_gen->used(), _old_gen->get_humongous_waste(), _heap->capacity());
+         _old_generation->used(), _old_generation->get_humongous_waste(), _heap->capacity());
   if (old_used > trigger_threshold) {
     _growth_trigger = true;
   }
@@ -620,13 +619,32 @@ void ShenandoahOldHeuristics::evaluate_triggers(size_t first_old_region, size_t
   set_trigger_if_old_is_overgrown();
 }
 
+bool ShenandoahOldHeuristics::should_resume_old_cycle() {
+  // If we are preparing to mark old, or if we are already marking old, then try to continue that work.
+  if (_old_generation->is_concurrent_mark_in_progress()) {
+    assert(_old_generation->state() == ShenandoahOldGeneration::MARKING, "Unexpected old gen state: %s", _old_generation->state_name());
+    log_trigger("Resume marking old");
+    return true;
+  }
+
+  if (_old_generation->is_preparing_for_mark()) {
+    assert(_old_generation->state() == ShenandoahOldGeneration::FILLING, "Unexpected old gen state: %s", _old_generation->state_name());
+    log_trigger("Resume preparing to mark old");
+    return true;
+  }
+
+  return false;
+}
+
 bool ShenandoahOldHeuristics::should_start_gc() {
-  // Cannot start a new old-gen GC until previous one has finished.
-  //
-  // Future refinement: under certain circumstances, we might be more sophisticated about this choice.
-  // For example, we could choose to abandon the previous old collection before it has completed evacuations.
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (!_old_generation->can_start_gc() || heap->collection_set()->has_old_regions()) {
+
+  const ShenandoahHeap* heap = ShenandoahHeap::heap();
+  if (_old_generation->is_doing_mixed_evacuations()) {
+    // Do not try to start an old cycle if we are waiting for old regions to be evacuated (we need
+    // a young cycle for this). Note that the young heuristic has a feature to expedite old evacuations.
+    // Future refinement: under certain circumstances, we might be more sophisticated about this choice.
+    // For example, we could choose to abandon the previous old collection before it has completed evacuations.
+    log_debug(gc)("Not starting an old cycle because we are waiting for mixed evacuations");
     return false;
   }
 

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

@@ -53,7 +53,6 @@ class ShenandoahOldHeuristics : public ShenandoahHeuristics {
   static uint NOT_FOUND;
 
   ShenandoahGenerationalHeap* _heap;
-  ShenandoahOldGeneration* _old_gen;
 
   // After final marking of the old generation, this heuristic will select
   // a set of candidate regions to be included in subsequent mixed collections.
@@ -186,6 +185,9 @@ class ShenandoahOldHeuristics : public ShenandoahHeuristics {
 
   bool should_start_gc() override;
 
+  // Returns true if the old generation needs to prepare for marking, or continue marking.
+  bool should_resume_old_cycle();
+
   void record_success_concurrent() override;
 
   void record_success_degenerated() override;

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

@@ -90,7 +90,7 @@ bool ShenandoahBarrierSet::need_keep_alive_barrier(DecoratorSet decorators, Basi
 void ShenandoahBarrierSet::on_slowpath_allocation_exit(JavaThread* thread, oop new_obj) {
 #if COMPILER2_OR_JVMCI
   assert(!ReduceInitialCardMarks || !ShenandoahCardBarrier || ShenandoahGenerationalHeap::heap()->is_in_young(new_obj),
-         "Error: losing card mark on initialzing store to old gen");
+         "Allocating new object outside of young generation: " INTPTR_FORMAT, p2i(new_obj));
 #endif // COMPILER2_OR_JVMCI
   assert(thread->deferred_card_mark().is_empty(), "We don't use this");
 }

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

@@ -123,25 +123,28 @@ void ShenandoahCollectorPolicy::record_shutdown() {
   _in_shutdown.set();
 }
 
-bool ShenandoahCollectorPolicy::is_at_shutdown() {
+bool ShenandoahCollectorPolicy::is_at_shutdown() const {
   return _in_shutdown.is_set();
 }
 
-bool is_explicit_gc(GCCause::Cause cause) {
+bool ShenandoahCollectorPolicy::is_explicit_gc(GCCause::Cause cause) {
   return GCCause::is_user_requested_gc(cause)
-      || GCCause::is_serviceability_requested_gc(cause);
+      || GCCause::is_serviceability_requested_gc(cause)
+      || cause == GCCause::_wb_full_gc
+      || cause == GCCause::_wb_young_gc;
 }
 
 bool is_implicit_gc(GCCause::Cause cause) {
   return cause != GCCause::_no_gc
       && cause != GCCause::_shenandoah_concurrent_gc
       && cause != GCCause::_allocation_failure
-      && !is_explicit_gc(cause);
+      && !ShenandoahCollectorPolicy::is_explicit_gc(cause);
 }
 
 #ifdef ASSERT
 bool is_valid_request(GCCause::Cause cause) {
-  return is_explicit_gc(cause)
+  return ShenandoahCollectorPolicy::is_explicit_gc(cause)
+      || ShenandoahCollectorPolicy::is_shenandoah_gc(cause)
       || cause == GCCause::_metadata_GC_clear_soft_refs
       || cause == GCCause::_codecache_GC_aggressive
       || cause == GCCause::_codecache_GC_threshold
@@ -153,6 +156,22 @@ bool is_valid_request(GCCause::Cause cause) {
 }
 #endif
 
+bool ShenandoahCollectorPolicy::is_shenandoah_gc(GCCause::Cause cause) {
+  return cause == GCCause::_allocation_failure
+      || cause == GCCause::_shenandoah_stop_vm
+      || cause == GCCause::_shenandoah_allocation_failure_evac
+      || cause == GCCause::_shenandoah_humongous_allocation_failure
+      || cause == GCCause::_shenandoah_concurrent_gc
+      || cause == GCCause::_shenandoah_upgrade_to_full_gc;
+}
+
+
+bool ShenandoahCollectorPolicy::is_allocation_failure(GCCause::Cause cause) {
+  return cause == GCCause::_allocation_failure
+      || cause == GCCause::_shenandoah_allocation_failure_evac
+      || cause == GCCause::_shenandoah_humongous_allocation_failure;
+}
+
 bool ShenandoahCollectorPolicy::is_requested_gc(GCCause::Cause cause) {
   return is_explicit_gc(cause) || is_implicit_gc(cause);
 }

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

@@ -77,9 +77,9 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
   void record_collection_cause(GCCause::Cause cause);
 
   void record_shutdown();
-  bool is_at_shutdown();
+  bool is_at_shutdown() const;
 
-  ShenandoahTracer* tracer() {return _tracer;}
+  ShenandoahTracer* tracer() const {return _tracer;}
 
   void print_gc_stats(outputStream* out) const;
 
@@ -90,15 +90,18 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
   // If the heuristics find that the number of consecutive degenerated cycles is above
   // ShenandoahFullGCThreshold, then they will initiate a Full GC upon an allocation
   // failure.
-  inline size_t consecutive_degenerated_gc_count() const {
+  size_t consecutive_degenerated_gc_count() const {
     return _consecutive_degenerated_gcs;
   }
 
+  static bool is_allocation_failure(GCCause::Cause cause);
+  static bool is_shenandoah_gc(GCCause::Cause cause);
   static bool is_requested_gc(GCCause::Cause cause);
+  static bool is_explicit_gc(GCCause::Cause cause);
   static bool should_run_full_gc(GCCause::Cause cause);
   static bool should_handle_requested_gc(GCCause::Cause cause);
 
-  inline size_t consecutive_young_gc_count() const {
+  size_t consecutive_young_gc_count() const {
     return _consecutive_young_gcs;
   }
 

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

@@ -50,7 +50,6 @@ ShenandoahControlThread::ShenandoahControlThread() :
 
 void ShenandoahControlThread::run_service() {
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
-
   const GCMode default_mode = concurrent_normal;
   const GCCause::Cause default_cause = GCCause::_shenandoah_concurrent_gc;
   int sleep = ShenandoahControlIntervalMin;
@@ -59,9 +58,14 @@ void ShenandoahControlThread::run_service() {
 
   ShenandoahCollectorPolicy* const policy = heap->shenandoah_policy();
   ShenandoahHeuristics* const heuristics = heap->heuristics();
-  while (!in_graceful_shutdown() && !should_terminate()) {
+  while (!should_terminate()) {
+    const GCCause::Cause cancelled_cause = heap->cancelled_cause();
+    if (cancelled_cause == GCCause::_shenandoah_stop_vm) {
+      break;
+    }
+
     // Figure out if we have pending requests.
-    const bool alloc_failure_pending = _alloc_failure_gc.is_set();
+    const bool alloc_failure_pending = ShenandoahCollectorPolicy::is_allocation_failure(cancelled_cause);
     const bool is_gc_requested = _gc_requested.is_set();
     const GCCause::Cause requested_gc_cause = _requested_gc_cause;
 
@@ -254,11 +258,6 @@ void ShenandoahControlThread::run_service() {
     }
     os::naked_short_sleep(sleep);
   }
-
-  // Wait for the actual stop(), can't leave run_service() earlier.
-  while (!should_terminate()) {
-    os::naked_short_sleep(ShenandoahControlIntervalMin);
-  }
 }
 
 void ShenandoahControlThread::service_concurrent_normal_cycle(GCCause::Cause cause) {
@@ -322,19 +321,24 @@ void ShenandoahControlThread::service_concurrent_normal_cycle(GCCause::Cause cau
 bool ShenandoahControlThread::check_cancellation_or_degen(ShenandoahGC::ShenandoahDegenPoint point) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   if (heap->cancelled_gc()) {
-    assert (is_alloc_failure_gc() || in_graceful_shutdown(), "Cancel GC either for alloc failure GC, or gracefully exiting");
-    if (!in_graceful_shutdown()) {
+    if (heap->cancelled_cause() == GCCause::_shenandoah_stop_vm) {
+      return true;
+    }
+
+    if (ShenandoahCollectorPolicy::is_allocation_failure(heap->cancelled_cause())) {
       assert (_degen_point == ShenandoahGC::_degenerated_outside_cycle,
               "Should not be set yet: %s", ShenandoahGC::degen_point_to_string(_degen_point));
       _degen_point = point;
+      return true;
     }
-    return true;
+
+    fatal("Unexpected reason for cancellation: %s", GCCause::to_string(heap->cancelled_cause()));
   }
   return false;
 }
 
 void ShenandoahControlThread::stop_service() {
-  // Nothing to do here.
+  ShenandoahHeap::heap()->cancel_gc(GCCause::_shenandoah_stop_vm);
 }
 
 void ShenandoahControlThread::service_stw_full_cycle(GCCause::Cause cause) {
@@ -363,6 +367,11 @@ void ShenandoahControlThread::request_gc(GCCause::Cause cause) {
 }
 
 void ShenandoahControlThread::handle_requested_gc(GCCause::Cause cause) {
+  if (should_terminate()) {
+    log_info(gc)("Control thread is terminating, no more GCs");
+    return;
+  }
+
   // 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
@@ -385,7 +394,7 @@ void ShenandoahControlThread::handle_requested_gc(GCCause::Cause cause) {
   MonitorLocker ml(&_gc_waiters_lock);
   size_t current_gc_id = get_gc_id();
   size_t required_gc_id = current_gc_id + 1;
-  while (current_gc_id < required_gc_id) {
+  while (current_gc_id < required_gc_id && !should_terminate()) {
     // Although setting gc request is under _gc_waiters_lock, but read side (run_service())
     // does not take the lock. We need to enforce following order, so that read side sees
     // latest requested gc cause when the flag is set.

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

@@ -25,6 +25,8 @@
 
 #include "gc/shared/gc_globals.hpp"
 #include "gc/shenandoah/shenandoahController.hpp"
+
+#include "shenandoahCollectorPolicy.hpp"
 #include "gc/shenandoah/shenandoahHeap.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.inline.hpp"
 
@@ -37,14 +39,6 @@ size_t ShenandoahController::reset_allocs_seen() {
   return Atomic::xchg(&_allocs_seen, (size_t)0, memory_order_relaxed);
 }
 
-void ShenandoahController::prepare_for_graceful_shutdown() {
-  _graceful_shutdown.set();
-}
-
-bool ShenandoahController::in_graceful_shutdown() {
-  return _graceful_shutdown.is_set();
-}
-
 void ShenandoahController::update_gc_id() {
   Atomic::inc(&_gc_id);
 }
@@ -53,59 +47,38 @@ size_t ShenandoahController::get_gc_id() {
   return Atomic::load(&_gc_id);
 }
 
-void ShenandoahController::handle_alloc_failure(ShenandoahAllocRequest& req, bool block) {
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-
+void ShenandoahController::handle_alloc_failure(const ShenandoahAllocRequest& req, bool block) {
   assert(current()->is_Java_thread(), "expect Java thread here");
-  bool is_humongous = ShenandoahHeapRegion::requires_humongous(req.size());
 
-  if (try_set_alloc_failure_gc(is_humongous)) {
-    // Only report the first allocation failure
-    log_info(gc)("Failed to allocate %s, %zu%s",
-                 req.type_string(),
-                 byte_size_in_proper_unit(req.size() * HeapWordSize), proper_unit_for_byte_size(req.size() * HeapWordSize));
+  const bool is_humongous = ShenandoahHeapRegion::requires_humongous(req.size());
+  const GCCause::Cause cause = is_humongous ? GCCause::_shenandoah_humongous_allocation_failure : GCCause::_allocation_failure;
 
-    // Now that alloc failure GC is scheduled, we can abort everything else
-    heap->cancel_gc(GCCause::_allocation_failure);
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  if (heap->cancel_gc(cause)) {
+    log_info(gc)("Failed to allocate %s, " PROPERFMT, req.type_string(), PROPERFMTARGS(req.size() * HeapWordSize));
+    request_gc(cause);
   }
 
-
   if (block) {
     MonitorLocker ml(&_alloc_failure_waiters_lock);
-    while (is_alloc_failure_gc()) {
+    while (!should_terminate() && ShenandoahCollectorPolicy::is_allocation_failure(heap->cancelled_cause())) {
       ml.wait();
     }
   }
 }
 
 void ShenandoahController::handle_alloc_failure_evac(size_t words) {
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  bool is_humongous = ShenandoahHeapRegion::requires_humongous(words);
 
-  if (try_set_alloc_failure_gc(is_humongous)) {
-    // Only report the first allocation failure
-    log_info(gc)("Failed to allocate %zu%s for evacuation",
-                 byte_size_in_proper_unit(words * HeapWordSize), proper_unit_for_byte_size(words * HeapWordSize));
-  }
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  const bool is_humongous = ShenandoahHeapRegion::requires_humongous(words);
+  const GCCause::Cause cause = is_humongous ? GCCause::_shenandoah_humongous_allocation_failure : GCCause::_shenandoah_allocation_failure_evac;
 
-  // Forcefully report allocation failure
-  heap->cancel_gc(GCCause::_shenandoah_allocation_failure_evac);
+  if (heap->cancel_gc(cause)) {
+    log_info(gc)("Failed to allocate " PROPERFMT " for evacuation", PROPERFMTARGS(words * HeapWordSize));
+  }
 }
 
 void ShenandoahController::notify_alloc_failure_waiters() {
-  _alloc_failure_gc.unset();
-  _humongous_alloc_failure_gc.unset();
   MonitorLocker ml(&_alloc_failure_waiters_lock);
   ml.notify_all();
 }
-
-bool ShenandoahController::try_set_alloc_failure_gc(bool is_humongous) {
-  if (is_humongous) {
-    _humongous_alloc_failure_gc.try_set();
-  }
-  return _alloc_failure_gc.try_set();
-}
-
-bool ShenandoahController::is_alloc_failure_gc() {
-  return _alloc_failure_gc.is_set();
-}