From 224fb5a3c41fe347d812d7499eb0eb34759533fc Mon Sep 17 00:00:00 2001 From: William Kemper Date: Tue, 16 Dec 2025 23:37:10 +0000 Subject: [PATCH 1/9] Instrumentation and fix --- .../shenandoahGenerationalControlThread.cpp | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index dfb28fd9ebb40..ef46224df6000 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -73,16 +73,24 @@ void ShenandoahGenerationalControlThread::run_service() { if (request.cause != GCCause::_no_gc) { run_gc_cycle(request); + log_debug(gc, thread)("After cycle, cancelled cause: %s", GCCause::to_string(_heap->cancelled_cause())); } // If the cycle was cancelled, continue the next iteration to deal with it. Otherwise, // if there was no other cycle requested, cleanup and wait for the next request. if (!_heap->cancelled_gc()) { MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); + log_debug(gc, thread)("After lock, cancelled cause: %s", GCCause::to_string(_heap->cancelled_cause())); if (_requested_gc_cause == GCCause::_no_gc) { set_gc_mode(ml, none); ml.wait(); + } else { + log_debug(gc, thread)("Not waiting because requested gc: %s, cancelled cause: %s", + GCCause::to_string(_requested_gc_cause), + GCCause::to_string(_heap->cancelled_cause())); } + } else { + log_debug(gc, thread)("Not waiting because cancelled gc: %s", GCCause::to_string(_heap->cancelled_cause())); } } @@ -111,22 +119,29 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& // the cancellation cause. request.cause = _heap->clear_cancellation(GCCause::_shenandoah_concurrent_gc); if (request.cause == GCCause::_shenandoah_concurrent_gc) { + log_debug(gc, thread)("Old gc was cancelled to run a young GC"); request.generation = _heap->young_generation(); + } else { + log_debug(gc, thread)("Using generation " PTR_FORMAT " on previous request", p2i(request.generation)); } } else { + if (_requested_generation == nullptr) { + log_debug(gc, thread)("Woke up to check for request (%s), but requested generation is nullptr", GCCause::to_string(_requested_gc_cause)); + } + request.cause = _requested_gc_cause; request.generation = _requested_generation; - - // Only clear these if we made a request from them. In the case of a cancelled gc, - // we do not want to inadvertently lose this pending request. - _requested_gc_cause = GCCause::_no_gc; - _requested_generation = nullptr; } + _requested_gc_cause = GCCause::_no_gc; + _requested_generation = nullptr; + log_debug(gc, thread)("Cleared pending request"); + if (request.cause == GCCause::_no_gc || request.cause == GCCause::_shenandoah_stop_vm) { return; } + assert(request.generation != nullptr, "Must know the generation here, cause is: %s", GCCause::to_string(request.cause)); GCMode mode; if (ShenandoahCollectorPolicy::is_allocation_failure(request.cause)) { mode = prepare_for_allocation_failure_gc(request); From 0037e21a0adec4d06785b4f1e9df48010523988b Mon Sep 17 00:00:00 2001 From: William Kemper Date: Tue, 16 Dec 2025 16:58:29 -0800 Subject: [PATCH 2/9] Remove instrumentation, add assertion and try a fix --- .../shenandoahGenerationalControlThread.cpp | 31 +++++++------------ .../jmxremote/startstop/REMOTE_TESTING.txt | 17 ---------- 2 files changed, 11 insertions(+), 37 deletions(-) delete mode 100644 test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index ef46224df6000..7b6298207d46a 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -73,24 +73,16 @@ void ShenandoahGenerationalControlThread::run_service() { if (request.cause != GCCause::_no_gc) { run_gc_cycle(request); - log_debug(gc, thread)("After cycle, cancelled cause: %s", GCCause::to_string(_heap->cancelled_cause())); } // If the cycle was cancelled, continue the next iteration to deal with it. Otherwise, // if there was no other cycle requested, cleanup and wait for the next request. if (!_heap->cancelled_gc()) { MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); - log_debug(gc, thread)("After lock, cancelled cause: %s", GCCause::to_string(_heap->cancelled_cause())); if (_requested_gc_cause == GCCause::_no_gc) { set_gc_mode(ml, none); ml.wait(); - } else { - log_debug(gc, thread)("Not waiting because requested gc: %s, cancelled cause: %s", - GCCause::to_string(_requested_gc_cause), - GCCause::to_string(_heap->cancelled_cause())); } - } else { - log_debug(gc, thread)("Not waiting because cancelled gc: %s", GCCause::to_string(_heap->cancelled_cause())); } } @@ -112,6 +104,10 @@ void ShenandoahGenerationalControlThread::stop_service() { void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& request) { // Hold the lock while we read request cause and generation MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); + + log_debug(gc, thread)("cancelled cause: %s, requested cause: %s", + GCCause::to_string(_heap->cancelled_cause()), GCCause::to_string(_requested_gc_cause)); + if (_heap->cancelled_gc()) { // The previous request was cancelled. Either it was cancelled for an allocation // failure (degenerated cycle), or old marking was cancelled to run a young collection. @@ -119,23 +115,21 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& // the cancellation cause. request.cause = _heap->clear_cancellation(GCCause::_shenandoah_concurrent_gc); if (request.cause == GCCause::_shenandoah_concurrent_gc) { - log_debug(gc, thread)("Old gc was cancelled to run a young GC"); request.generation = _heap->young_generation(); - } else { - log_debug(gc, thread)("Using generation " PTR_FORMAT " on previous request", p2i(request.generation)); + } else if (_degen_point == ShenandoahGC::_degenerated_unset) { + _degen_point = ShenandoahGC::_degenerated_outside_cycle; + request.generation = _heap->young_generation(); } } else { - if (_requested_generation == nullptr) { - log_debug(gc, thread)("Woke up to check for request (%s), but requested generation is nullptr", GCCause::to_string(_requested_gc_cause)); - } - request.cause = _requested_gc_cause; request.generation = _requested_generation; } + log_debug(gc, thread)("request.cause: %s, request.generation: %s", + GCCause::to_string(request.cause), request.generation == nullptr ? "None" : request.generation->name()); + _requested_gc_cause = GCCause::_no_gc; _requested_generation = nullptr; - log_debug(gc, thread)("Cleared pending request"); if (request.cause == GCCause::_no_gc || request.cause == GCCause::_shenandoah_stop_vm) { return; @@ -156,10 +150,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(); - } else if (request.generation->is_old()) { + if (request.generation->is_old()) { // This means we degenerated during the young bootstrap for the old generation // cycle. The following degenerated cycle should therefore also be young. request.generation = _heap->young_generation(); diff --git a/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt b/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt deleted file mode 100644 index a8f7b1527c7d8..0000000000000 --- a/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt +++ /dev/null @@ -1,17 +0,0 @@ -1. Setup two hosts -2. Make sure tcp connection between them works -3. run tcpdump -i host and 'tcp[13] & 2!=0' - on host 1 -4. run - - ${TESTJAVA}/bin/java -server TestApp \ - -Dcom.sun.management.jmxremote.port=50234 \ - -Dcom.sun.management.jmxremote.rmi.port=50235 \ - -Dcom.sun.management.jmxremote.authenticate=false \ - -Dcom.sun.management.jmxremote.ssl=false - - on host2 -5. run jconsole on host1 -6. connect jconsole to host2:50234 - Make sure jconsole works - Make sure only host2.50234 and host2.50235 appears in tcpdump output. From 395d6e05c540e9b5d70d0ca76e033e00e03c0526 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Wed, 17 Dec 2025 11:24:16 -0800 Subject: [PATCH 3/9] Try to simplify control thread protocol --- .../shenandoahGenerationalControlThread.cpp | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index 7b6298207d46a..70fe046b333e5 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -77,12 +77,10 @@ void ShenandoahGenerationalControlThread::run_service() { // If the cycle was cancelled, continue the next iteration to deal with it. Otherwise, // if there was no other cycle requested, cleanup and wait for the next request. - if (!_heap->cancelled_gc()) { - MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); - if (_requested_gc_cause == GCCause::_no_gc) { - set_gc_mode(ml, none); - ml.wait(); - } + MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); + if (_requested_gc_cause == GCCause::_no_gc) { + set_gc_mode(ml, none); + ml.wait(); } } @@ -108,20 +106,20 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& log_debug(gc, thread)("cancelled cause: %s, requested cause: %s", GCCause::to_string(_heap->cancelled_cause()), GCCause::to_string(_requested_gc_cause)); - if (_heap->cancelled_gc()) { - // The previous request was cancelled. Either it was cancelled for an allocation - // failure (degenerated cycle), or old marking was cancelled to run a young collection. - // In either case, the correct generation for the next cycle can be determined by - // the cancellation cause. - request.cause = _heap->clear_cancellation(GCCause::_shenandoah_concurrent_gc); - if (request.cause == GCCause::_shenandoah_concurrent_gc) { - request.generation = _heap->young_generation(); - } else if (_degen_point == ShenandoahGC::_degenerated_unset) { - _degen_point = ShenandoahGC::_degenerated_outside_cycle; + request.cause = _requested_gc_cause; + if (ShenandoahCollectorPolicy::is_allocation_failure(request.cause)) { + if (_degen_point == ShenandoahGC::_degenerated_outside_cycle) { request.generation = _heap->young_generation(); + } else { + assert(request.generation != nullptr, "Must know which generation to use for degenerated cycle"); } } else { - request.cause = _requested_gc_cause; + if (request.cause == GCCause::_shenandoah_concurrent_gc) { + // This is a regulator request. It must have request generation set. It is also + // possible that the regulator "canceled" an old mark, so we can clear that. + // This clear operation will only clear the cancellation if it a regulator request. + _heap->clear_cancellation(GCCause::_shenandoah_concurrent_gc); + } request.generation = _requested_generation; } @@ -657,7 +655,8 @@ bool ShenandoahGenerationalControlThread::request_concurrent_gc(ShenandoahGenera MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); if (gc_mode() == servicing_old) { if (!preempt_old_marking(generation)) { - log_debug(gc, thread)("Cannot start young, old collection is not preemptible"); + // Global should be able to cause old collection to be abandoned + log_debug(gc, thread)("Cannot start %s, old collection is not preemptible", generation->name()); return false; } @@ -665,7 +664,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_control_thread(ml, GCCause::_shenandoah_concurrent_gc, generation); ml.wait(); } return true; From 53dcc61e89c0217cf1e245b819fce537dc96e69a Mon Sep 17 00:00:00 2001 From: William Kemper Date: Wed, 17 Dec 2025 11:53:41 -0800 Subject: [PATCH 4/9] Fix degen point handling --- .../gc/shenandoah/shenandoahGenerationalControlThread.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index 70fe046b333e5..88d7d6a393dd5 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -62,6 +62,7 @@ ShenandoahGenerationalControlThread::ShenandoahGenerationalControlThread() : void ShenandoahGenerationalControlThread::run_service() { ShenandoahGCRequest request; + request.generation = _heap->young_generation(); while (!should_terminate()) { // Figure out if we have pending requests. @@ -108,8 +109,9 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& request.cause = _requested_gc_cause; if (ShenandoahCollectorPolicy::is_allocation_failure(request.cause)) { - if (_degen_point == ShenandoahGC::_degenerated_outside_cycle) { + if (_degen_point == ShenandoahGC::_degenerated_unset) { request.generation = _heap->young_generation(); + _degen_point = ShenandoahGC::_degenerated_outside_cycle; } else { assert(request.generation != nullptr, "Must know which generation to use for degenerated cycle"); } From f64fedf5c455cf3824da8c04ad368f835776b199 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Wed, 17 Dec 2025 13:57:13 -0800 Subject: [PATCH 5/9] Do not let allocation failure requests be overwritten by other requests --- .../shenandoahGenerationalControlThread.cpp | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index 88d7d6a393dd5..24cc6079f0215 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -117,9 +117,9 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& } } else { if (request.cause == GCCause::_shenandoah_concurrent_gc) { - // This is a regulator request. It must have request generation set. It is also - // possible that the regulator "canceled" an old mark, so we can clear that. - // This clear operation will only clear the cancellation if it a regulator request. + // This is a regulator request. It is also possible that the regulator "canceled" an old mark, + // so we can clear that here. This clear operation will only clear the cancellation if it is + // a regulator request. _heap->clear_cancellation(GCCause::_shenandoah_concurrent_gc); } request.generation = _requested_generation; @@ -135,7 +135,7 @@ void ShenandoahGenerationalControlThread::check_for_request(ShenandoahGCRequest& return; } - assert(request.generation != nullptr, "Must know the generation here, cause is: %s", GCCause::to_string(request.cause)); + assert(request.generation != nullptr, "request.generation cannot be null, cause is: %s", GCCause::to_string(request.cause)); GCMode mode; if (ShenandoahCollectorPolicy::is_allocation_failure(request.cause)) { mode = prepare_for_allocation_failure_gc(request); @@ -700,10 +700,14 @@ void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause c void ShenandoahGenerationalControlThread::notify_control_thread(MonitorLocker& ml, GCCause::Cause cause, ShenandoahGeneration* generation) { assert(_control_lock.is_locked(), "Request lock must be held here"); - log_debug(gc, thread)("Notify control (%s): %s, %s", gc_mode_name(gc_mode()), GCCause::to_string(cause), generation->name()); - _requested_gc_cause = cause; - _requested_generation = generation; - ml.notify(); + if (ShenandoahCollectorPolicy::is_allocation_failure(_requested_gc_cause)) { + log_debug(gc, thread)("Not overwriting gc cause %s with %s", GCCause::to_string(_requested_gc_cause), GCCause::to_string(cause)); + } else { + log_debug(gc, thread)("Notify control (%s): %s, %s", gc_mode_name(gc_mode()), GCCause::to_string(cause), generation->name()); + _requested_gc_cause = cause; + _requested_generation = generation; + ml.notify(); + } } void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause cause) { @@ -713,9 +717,13 @@ void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause c 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; - ml.notify(); + if (ShenandoahCollectorPolicy::is_allocation_failure(_requested_gc_cause)) { + log_debug(gc, thread)("Not overwriting gc cause %s with %s", GCCause::to_string(_requested_gc_cause), GCCause::to_string(cause)); + } else { + log_debug(gc, thread)("Notify control (%s): %s", gc_mode_name(gc_mode()), GCCause::to_string(cause)); + _requested_gc_cause = cause; + ml.notify(); + } } bool ShenandoahGenerationalControlThread::preempt_old_marking(ShenandoahGeneration* generation) { From 31df868745390577b5897bea2eb04b07843da3ff Mon Sep 17 00:00:00 2001 From: William Kemper Date: Wed, 17 Dec 2025 17:01:05 -0800 Subject: [PATCH 6/9] Carry over gc cancellation to gc request --- .../share/gc/shenandoah/shenandoahGenerationalControlThread.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index 24cc6079f0215..428da467b625f 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -594,6 +594,8 @@ bool ShenandoahGenerationalControlThread::check_cancellation_or_degen(Shenandoah if (ShenandoahCollectorPolicy::is_allocation_failure(_heap->cancelled_cause())) { assert(_degen_point == ShenandoahGC::_degenerated_unset, "Should not be set yet: %s", ShenandoahGC::degen_point_to_string(_degen_point)); + MonitorLocker ml(&_control_lock, Mutex::_no_safepoint_check_flag); + _requested_gc_cause = _heap->cancelled_cause(); _degen_point = point; log_debug(gc, thread)("Cancellation detected:, reason: %s, degen point: %s", GCCause::to_string(_heap->cancelled_cause()), From bd42dc665cd2444e9fc802edd00cba5fb2c2fd52 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Fri, 19 Dec 2025 10:41:15 -0800 Subject: [PATCH 7/9] Don't know how this file got deleted --- .../jmxremote/startstop/REMOTE_TESTING.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt diff --git a/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt b/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt new file mode 100644 index 0000000000000..a8f7b1527c7d8 --- /dev/null +++ b/test/jdk/sun/management/jmxremote/startstop/REMOTE_TESTING.txt @@ -0,0 +1,17 @@ +1. Setup two hosts +2. Make sure tcp connection between them works +3. run tcpdump -i host and 'tcp[13] & 2!=0' + on host 1 +4. run + + ${TESTJAVA}/bin/java -server TestApp \ + -Dcom.sun.management.jmxremote.port=50234 \ + -Dcom.sun.management.jmxremote.rmi.port=50235 \ + -Dcom.sun.management.jmxremote.authenticate=false \ + -Dcom.sun.management.jmxremote.ssl=false + + on host2 +5. run jconsole on host1 +6. connect jconsole to host2:50234 + Make sure jconsole works + Make sure only host2.50234 and host2.50235 appears in tcpdump output. From cfd167f5a883df33953a663185d1bd0d6e74132f Mon Sep 17 00:00:00 2001 From: William Kemper Date: Fri, 19 Dec 2025 10:55:18 -0800 Subject: [PATCH 8/9] Revert back to what should be on this branch --- .../shenandoahGenerationalControlThread.hpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp index b7dbedd5e8461..13e69d2526870 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.hpp @@ -135,16 +135,13 @@ class ShenandoahGenerationalControlThread: public ShenandoahController { // Return printable name for the given gc mode. static const char* gc_mode_name(GCMode mode); - // 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. + // 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); 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(); From 8b3acc854e965f0d77e3db1204644f9540722318 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Fri, 19 Dec 2025 10:55:31 -0800 Subject: [PATCH 9/9] Add comments --- .../gc/shenandoah/shenandoahGenerationalControlThread.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp index 428da467b625f..018b4898a1974 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalControlThread.cpp @@ -61,6 +61,10 @@ ShenandoahGenerationalControlThread::ShenandoahGenerationalControlThread() : void ShenandoahGenerationalControlThread::run_service() { + // This is the only instance of request. It is important that request.generation + // does not change between a concurrent cycle failure and the start of a degenerated + // cycle. We initialize it with the young generation to handle the pathological case + // where the very first cycle is degenerated (some tests exercise this path). ShenandoahGCRequest request; request.generation = _heap->young_generation(); while (!should_terminate()) { @@ -703,6 +707,8 @@ void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause c void ShenandoahGenerationalControlThread::notify_control_thread(MonitorLocker& ml, GCCause::Cause cause, ShenandoahGeneration* generation) { assert(_control_lock.is_locked(), "Request lock must be held here"); if (ShenandoahCollectorPolicy::is_allocation_failure(_requested_gc_cause)) { + // We have already observed a request to handle an allocation failure. We cannot allow + // another request (System.gc or regulator) to subvert the degenerated cycle. log_debug(gc, thread)("Not overwriting gc cause %s with %s", GCCause::to_string(_requested_gc_cause), GCCause::to_string(cause)); } else { log_debug(gc, thread)("Notify control (%s): %s, %s", gc_mode_name(gc_mode()), GCCause::to_string(cause), generation->name()); @@ -720,6 +726,8 @@ void ShenandoahGenerationalControlThread::notify_control_thread(GCCause::Cause c void ShenandoahGenerationalControlThread::notify_control_thread(MonitorLocker& ml, GCCause::Cause cause) { assert(_control_lock.is_locked(), "Request lock must be held here"); if (ShenandoahCollectorPolicy::is_allocation_failure(_requested_gc_cause)) { + // We have already observed a request to handle an allocation failure. We cannot allow + // another request (System.gc or regulator) to subvert the degenerated cycle. log_debug(gc, thread)("Not overwriting gc cause %s with %s", GCCause::to_string(_requested_gc_cause), GCCause::to_string(cause)); } else { log_debug(gc, thread)("Notify control (%s): %s", gc_mode_name(gc_mode()), GCCause::to_string(cause));