8374048: Genshen: Backout fix for missed cancellation notice

Reviewed-by: kdnilsen, ysr

Author: William Kemper

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

@@ -96,7 +96,8 @@ void ShenandoahGenerationalControlThread::stop_service() {
   log_debug(gc, thread)("Stopping control thread");
   MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag);
   _heap->cancel_gc(GCCause::_shenandoah_stop_vm);
-  notify_control_thread(ml, GCCause::_shenandoah_stop_vm);
+  _requested_gc_cause = GCCause::_shenandoah_stop_vm;
+  notify_cancellation(ml, GCCause::_shenandoah_stop_vm);
   // We can't wait here because it may interfere with the active cycle's ability
   // to reach a safepoint (this runs on a java thread).
 }
@@ -139,8 +140,7 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest&
 }
 
 ShenandoahGenerationalControlThread::GCMode ShenandoahGenerationalControlThread::prepare_for_allocation_failure_gc(ShenandoahGCRequest &request) {
-  // Important: not all paths update the request.generation. This is intentional.
-  // A degenerated cycle must use the same generation carried over from the previous request.
+
   if (_degen_point == ShenandoahGC::_degenerated_unset) {
     _degen_point = ShenandoahGC::_degenerated_outside_cycle;
     request.generation = _heap->young_generation();
@@ -633,7 +633,9 @@ void ShenandoahGenerationalControlThread::service_stw_degenerated_cycle(const Sh
 
 void ShenandoahGenerationalControlThread::request_gc(GCCause::Cause cause) {
   if (ShenandoahCollectorPolicy::is_allocation_failure(cause)) {
-    notify_control_thread(cause);
+    // GC should already be cancelled. Here we are just notifying the control thread to
+    // wake up and handle the cancellation request, so we don't need to set _requested_gc_cause.
+    notify_cancellation(cause);
   } else if (ShenandoahCollectorPolicy::should_handle_requested_gc(cause)) {
     handle_requested_gc(cause);
   }
@@ -659,7 +661,7 @@ bool ShenandoahGenerationalControlThread::request_concurrent_gc(ShenandoahGenera
     log_info(gc)("Preempting old generation mark to allow %s GC", generation->name());
     while (gc_mode() == servicing_old) {
       ShenandoahHeap::heap()->cancel_gc(GCCause::_shenandoah_concurrent_gc);
-      notify_control_thread(ml, GCCause::_shenandoah_concurrent_gc);
+      notify_cancellation(ml, GCCause::_shenandoah_concurrent_gc);
       ml.wait();
     }
     return true;
@@ -699,15 +701,14 @@ void ShenandoahGenerationalControlThread::notify_control_thread(MonitorLocker& m
   ml.notify();
 }
 
-void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause cause) {
+void ShenandoahGenerationalControlThread::notify_cancellation(GCCause::Cause cause) {
   MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag);
-  notify_control_thread(ml, cause);
+  notify_cancellation(ml, cause);
 }
 
-void ShenandoahGenerationalControlThread::notify_control_thread(MonitorLocker& ml, GCCause::Cause cause) {
-  assert(_control_lock.is_locked(), "Request lock must be held here");
-  log_debug(gc, thread)("Notify control (%s): %s", gc_mode_name(gc_mode()), GCCause::to_string(cause));
-  _requested_gc_cause = cause;
+void ShenandoahGenerationalControlThread::notify_cancellation(MonitorLocker& ml, GCCause::Cause cause) {
+  assert(_heap->cancelled_gc(), "GC should already be cancelled");
+  log_debug(gc,thread)("Notify control (%s): %s", gc_mode_name(gc_mode()), GCCause::to_string(cause));
   ml.notify();
 }
 

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

@@ -135,13 +135,16 @@ class ShenandoahGenerationalControlThread: public ShenandoahController {
   // Return printable name for the given gc mode.
   static const char* gc_mode_name(GCMode mode);
 
-  // These notify the control thread after updating _requested_gc_cause and (optionally) _requested_generation.
-  // Updating the requested generation is not necessary for allocation failures nor when stopping the thread.
-  void notify_control_thread(GCCause::Cause cause);
-  void notify_control_thread(MonitorLocker& ml, GCCause::Cause cause);
+  // Takes the request lock and updates the requested cause and generation, then notifies the control thread.
+  // The overloaded variant should be used when the _control_lock is already held.
   void notify_control_thread(GCCause::Cause cause, ShenandoahGeneration* generation);
   void notify_control_thread(MonitorLocker& ml, GCCause::Cause cause, ShenandoahGeneration* generation);
 
+  // Notifies the control thread, but does not update the requested cause or generation.
+  // The overloaded variant should be used when the _control_lock is already held.
+  void notify_cancellation(GCCause::Cause cause);
+  void notify_cancellation(MonitorLocker& ml, GCCause::Cause cause);
+
   // Configure the heap to age objects and regions if the aging period has elapsed.
   void maybe_set_aging_cycle();