8389096

Hi all,

  please review this fix to avoid deadlocks (or crashes in earlier VM versions) when we want to start GCs with `GCALotAtAllSafepoints` (and `ScavengeALot`).

There are actually two causes:
* GCs that use the GCLocker do not allow starting GCs when it is held
* the other issue is the GC start when holding the `Heap_lock`; starting a new GC will automatically try to grab it (recursively), which HotSpot does not support, resulting in a hagn too

The change
* does not start the GC-a-lot procedure when we are in a critical section (`JavaThread::in_critical()` is only set for collectors using it, so others will just correctly advance)
* when waiting for the `Heap_lock` the VM also suppresses (only) the GC invocation. This is only the case for the `JVM_WaitForReferencePendingList` call.

Arguably we may want to disable GC when waiting for any lock. This is up for discussion, but it works with just suppressing them with the `Heap_lock` held.

Testing:
test case does not hang any more.  It's not a 100% reproducer, but something like 75%. I opted for simplicity than reproducability here.

Thanks,
  Thomas

Author: tschatzl

src/hotspot/share/runtime/interfaceSupport.cpp

@@ -85,10 +85,14 @@ unsigned int InterfaceSupport::_fullgc_alot_counter   = 1;
 intx InterfaceSupport::_fullgc_alot_invocation = 0;
 
 void InterfaceSupport::gc_alot() {
-  Thread *thread = Thread::current();
+  Thread* thread = Thread::current();
   if (!thread->is_Java_thread()) return; // Avoid concurrent calls
+  JavaThread* current_thread = JavaThread::cast(thread);
+  // If we are in a critical section, collectors deadlock when trying to start a GC.
+  // In_critical() is only set for those collectors where this is the case. Others
+  // like G1 do not and have no problem garbage collecting here.
+  if (current_thread->in_critical()) return;
   // Check for new, not quite initialized thread. A thread in new mode cannot initiate a GC.
-  JavaThread *current_thread = JavaThread::cast(thread);
   if (current_thread->active_handles() == nullptr) return;
 
   // Short-circuit any possible re-entrant gc-a-lot attempt

src/hotspot/share/runtime/javaThread.cpp

@@ -281,7 +281,7 @@ void JavaThread::check_possible_safepoint() {
 #endif // CHECK_UNHANDLED_OOPS
 }
 
-void JavaThread::check_for_valid_safepoint_state() {
+void JavaThread::check_for_valid_safepoint_state(bool allow_gcalot) {
   // Don't complain if running a debugging command.
   if (DebuggingContext::is_enabled()) return;
 
@@ -294,7 +294,7 @@ void JavaThread::check_for_valid_safepoint_state() {
     fatal("LEAF method calling lock?");
   }
 
-  if (GCALotAtAllSafepoints) {
+  if (GCALotAtAllSafepoints && allow_gcalot) {
     // We could enter a safepoint here and thus have a gc
     InterfaceSupport::check_gc_alot();
   }

src/hotspot/share/runtime/javaThread.hpp

@@ -284,8 +284,8 @@ class JavaThread: public Thread {
  public:
   // These functions check conditions before possibly going to a safepoint.
   // including NoSafepointVerifier.
-  void check_for_valid_safepoint_state() NOT_DEBUG_RETURN;
-  void check_possible_safepoint()        NOT_DEBUG_RETURN;
+  void check_for_valid_safepoint_state(bool allow_gcalot = true) NOT_DEBUG_RETURN;
+  void check_possible_safepoint()                                NOT_DEBUG_RETURN;
 
 #ifdef ASSERT
  private:

src/hotspot/share/runtime/mutex.cpp

@@ -61,7 +61,7 @@ void Mutex::check_block_state(Thread* thread) {
          "locking not allowed when crash protection is set");
 }
 
-void Mutex::check_safepoint_state(Thread* thread) {
+void Mutex::check_safepoint_state(Thread* thread, bool allow_gcalot) {
   check_block_state(thread);
 
   // If the lock acquisition checks for safepoint, verify that the lock was created with rank that
@@ -72,7 +72,7 @@ void Mutex::check_safepoint_state(Thread* thread) {
 
   if (thread->is_active_Java_thread()) {
     // Also check NoSafepointVerifier, and thread state is _thread_in_vm
-    JavaThread::cast(thread)->check_for_valid_safepoint_state();
+    JavaThread::cast(thread)->check_for_valid_safepoint_state(allow_gcalot);
   }
 }
 
@@ -116,7 +116,7 @@ void Mutex::lock_contended(Thread* self) {
 void Mutex::lock(Thread* self) {
   assert(owner() != self, "invariant");
 
-  check_safepoint_state(self);
+  check_safepoint_state(self, true /* allow_gcalot */);
   check_rank(self);
 
   OrderAccess::fence();
@@ -245,7 +245,9 @@ bool Monitor::wait(uint64_t timeout) {
   set_owner(nullptr);
 
   // Check safepoint state after resetting owner and possible NSV.
-  check_safepoint_state(self);
+  // Don't do GCALot verification here: the lock is held until the wait() below, and if
+  // this lock is Heap_lock, we would deadlock.
+  check_safepoint_state(self, this != Heap_lock);
 
   int wait_status;
   InFlightMutexRelease ifmr(this);

src/hotspot/share/runtime/mutex.hpp

@@ -141,7 +141,7 @@ class Mutex : public CHeapObj<mtSynchronizer> {
  protected:
   void set_owner_implementation(Thread* owner)                        NOT_DEBUG({ raw_set_owner(owner);});
   void check_block_state       (Thread* thread)                       NOT_DEBUG_RETURN;
-  void check_safepoint_state   (Thread* thread)                       NOT_DEBUG_RETURN;
+  void check_safepoint_state   (Thread* thread, bool allow_gcalot)    NOT_DEBUG_RETURN;
   void check_no_safepoint_state(Thread* thread)                       NOT_DEBUG_RETURN;
   void check_rank              (Thread* thread)                       NOT_DEBUG_RETURN;
   void assert_owner            (Thread* expected)                     NOT_DEBUG_RETURN;

test/hotspot/jtreg/gc/TestGCALotAtAllSafepoints.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package gc;
+
+/**
+ * @test id=Serial
+ * @bug 8389096
+ * @summary Verify that -XX:GCALotAtAllSafepoints and -XX:+ScavengeALot do not hang the VM.
+ * @comment GCALotAtAllSafepoints and ScavengeALot cause garbage collections at many places in the VM. These
+ *          garbage collection should not cause hangs.
+ * @requires vm.flagless
+ * @requires vm.debug
+ * @requires vm.gc.Serial
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ * @run driver/timeout=60 gc.TestGCALotAtAllSafepoints -XX:+UseSerialGC
+ */
+
+/**
+ * @test id=Parallel
+ * @bug 8389096
+ * @summary Verify that -XX:GCALotAtAllSafepoints and -XX:+ScavengeALot do not hang the VM.
+ * @comment GCALotAtAllSafepoints and ScavengeALot cause garbage collections at many places in the VM. These
+ *          garbage collection should not cause hangs.
+ * @requires vm.flagless
+ * @requires vm.debug
+ * @requires vm.gc.Parallel
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ * @run driver/timeout=60 gc.TestGCALotAtAllSafepoints -XX:+UseParallelGC
+ */
+
+/**
+ * @test id=G1
+ * @bug 8389096
+ * @summary Verify that -XX:GCALotAtAllSafepoints and -XX:+ScavengeALot do not hang the VM.
+ * @comment GCALotAtAllSafepoints and ScavengeALot cause garbage collections at many places in the VM. These
+ *          garbage collection should not cause hangs.
+ * @requires vm.flagless
+ * @requires vm.debug
+ * @requires vm.gc.G1
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ * @run driver/timeout=60 gc.TestGCALotAtAllSafepoints -XX:+UseG1GC
+ */
+
+/**
+ * @test id=Z
+ * @bug 8389096
+ * @summary Verify that -XX:GCALotAtAllSafepoints and -XX:+ScavengeALot do not hang the VM.
+ * @comment GCALotAtAllSafepoints and ScavengeALot cause garbage collections at many places in the VM. These
+ *          garbage collection should not cause hangs.
+ * @requires vm.flagless
+ * @requires vm.debug
+ * @requires vm.gc.Z
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ * @run driver/timeout=60 gc.TestGCALotAtAllSafepoints -XX:+UseZGC
+ */
+
+/**
+ * @test id=Shenandoah
+ * @bug 8389096
+ * @summary Verify that -XX:GCALotAtAllSafepoints and -XX:+ScavengeALot do not hang the VM.
+ * @comment GCALotAtAllSafepoints and ScavengeALot cause garbage collections at many places in the VM. These
+ *          garbage collection should not cause hangs.
+ * @requires vm.flagless
+ * @requires vm.debug
+ * @requires vm.gc.Shenandoah
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ * @run driver/timeout=60 gc.TestGCALotAtAllSafepoints -XX:+UseShenandoahGC
+ */
+
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class TestGCALotAtAllSafepoints {
+    public static void main(String[] args) throws Exception {
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(args[0],
+                                                                             "-Xmx16m",
+                                                                             "-XX:+GCALotAtAllSafepoints",
+                                                                             "-XX:+ScavengeALot",
+                                                                             "NoSuchClass");
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        output.shouldMatch("Error: Could not find or load main class NoSuchClass");
+        output.shouldHaveExitValue(1);
+    }
+}