From 7ac72d18c2ad9839c4ebcda346cb604135aece49 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Mon, 6 Jul 2026 08:58:39 +0000 Subject: [PATCH] 8225186: G1: Compiler code cache requested GC deadlocks while WhiteBox has control Reviewed-by: ayang, iwalulya --- src/hotspot/share/code/codeCache.cpp | 27 ++- src/hotspot/share/code/codeCache.hpp | 10 +- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 24 +- src/hotspot/share/gc/g1/g1Policy.cpp | 6 +- src/hotspot/share/gc/g1/g1VMOperations.cpp | 28 ++- src/hotspot/share/gc/g1/g1VMOperations.hpp | 4 +- .../gc/shared/concurrentGCBreakpoints.cpp | 4 +- .../jtreg/gc/g1/TestCodeCacheWhiteBox.java | 216 ++++++++++++++++++ 8 files changed, 291 insertions(+), 28 deletions(-) create mode 100644 test/hotspot/jtreg/gc/g1/TestCodeCacheWhiteBox.java diff --git a/src/hotspot/share/code/codeCache.cpp b/src/hotspot/share/code/codeCache.cpp index f1ca317d36f..94cf8ebdec1 100644 --- a/src/hotspot/share/code/codeCache.cpp +++ b/src/hotspot/share/code/codeCache.cpp @@ -38,6 +38,7 @@ #include "gc/shared/barrierSetNMethod.hpp" #include "gc/shared/classUnloadingContext.hpp" #include "gc/shared/collectedHeap.hpp" +#include "gc/shared/gcCause.hpp" #include "jfr/jfrEvents.hpp" #include "jvm_io.h" #include "logging/log.hpp" @@ -814,7 +815,8 @@ void CodeCache::update_cold_gc_count() { size_t used = max - free; double gc_interval = time - last_time; - _unloading_threshold_gc_requested = false; + AtomicAccess::store(&_unloading_threshold_gc_state, UnloadingRequestState::Idle); + _last_unloading_time = time; _last_unloading_used = used; @@ -889,7 +891,7 @@ void CodeCache::gc_on_allocation() { double free_ratio = double(free) / double(max); if (free_ratio <= StartAggressiveSweepingAt / 100.0) { // In case the GC is concurrent, we make sure only one thread requests the GC. - if (AtomicAccess::cmpxchg(&_unloading_threshold_gc_requested, false, true) == false) { + if (AtomicAccess::cmpxchg(&_unloading_threshold_gc_state, UnloadingRequestState::Idle, UnloadingRequestState::Active) == UnloadingRequestState::Idle) { log_info(codecache)("Triggering aggressive GC due to having only %.3f%% free memory", free_ratio * 100.0); Universe::heap()->collect(GCCause::_codecache_GC_aggressive); } @@ -915,7 +917,7 @@ void CodeCache::gc_on_allocation() { // it is eventually invoked to avoid trouble. if (allocated_since_last_ratio > threshold) { // In case the GC is concurrent, we make sure only one thread requests the GC. - if (AtomicAccess::cmpxchg(&_unloading_threshold_gc_requested, false, true) == false) { + if (AtomicAccess::cmpxchg(&_unloading_threshold_gc_state, UnloadingRequestState::Idle, UnloadingRequestState::Active) == UnloadingRequestState::Idle) { log_info(codecache)("Triggering threshold (%.3f%%) GC due to allocating %.3f%% since last unloading (%.3f%% used -> %.3f%% used)", threshold * 100.0, allocated_since_last_ratio * 100.0, last_used_ratio * 100.0, used_ratio * 100.0); Universe::heap()->collect(GCCause::_codecache_GC_threshold); @@ -935,7 +937,7 @@ uint64_t CodeCache::_cold_gc_count = INT_MAX; double CodeCache::_last_unloading_time = 0.0; size_t CodeCache::_last_unloading_used = 0; -volatile bool CodeCache::_unloading_threshold_gc_requested = false; +volatile CodeCache::UnloadingRequestState CodeCache::_unloading_threshold_gc_state = UnloadingRequestState::Idle; TruncatedSeq CodeCache::_unloading_gc_intervals(10 /* samples */); TruncatedSeq CodeCache::_unloading_allocation_rates(10 /* samples */); @@ -970,6 +972,23 @@ void CodeCache::on_gc_marking_cycle_finish() { update_cold_gc_count(); } +void CodeCache::defer_unloading_gc_request() { + assert_at_safepoint(); + assert(_unloading_threshold_gc_state == UnloadingRequestState::Active, "only defer active requests"); + AtomicAccess::store(&_unloading_threshold_gc_state, UnloadingRequestState::Deferred); +} + +void CodeCache::clear_deferred_unloading_gc_request() { + // Codecache marking may still be active after aborting gc marking, so we can not + // use is_marking_active() to check whether we are in the correct state to clear + // the deferred state. + // Requests are only deferred outside GC marking, and only cleared after + // at the end of whitebox, we can just clear it if it was Deferred. + AtomicAccess::cmpxchg(&_unloading_threshold_gc_state, + UnloadingRequestState::Deferred, + UnloadingRequestState::Idle); +} + void CodeCache::arm_all_nmethods() { BarrierSet::barrier_set()->barrier_set_nmethod()->arm_all_nmethods(); } diff --git a/src/hotspot/share/code/codeCache.hpp b/src/hotspot/share/code/codeCache.hpp index 3b8aa5b2e58..bef114e5e19 100644 --- a/src/hotspot/share/code/codeCache.hpp +++ b/src/hotspot/share/code/codeCache.hpp @@ -107,7 +107,12 @@ class CodeCache : AllStatic { static double _last_unloading_time; static TruncatedSeq _unloading_gc_intervals; static TruncatedSeq _unloading_allocation_rates; - static volatile bool _unloading_threshold_gc_requested; + enum UnloadingRequestState : uint { + Idle, + Active, + Deferred + }; + static volatile UnloadingRequestState _unloading_threshold_gc_state; static ExceptionCache* volatile _exception_cache_purge_list; @@ -198,6 +203,9 @@ class CodeCache : AllStatic { static uint64_t previous_completed_gc_marking_cycle(); static void on_gc_marking_cycle_start(); static void on_gc_marking_cycle_finish(); + + static void defer_unloading_gc_request(); + static void clear_deferred_unloading_gc_request(); // Arm nmethods so that special actions are taken (nmethod_entry_barrier) for // on-stack nmethods. It's used in two places: // 1. Used before the start of concurrent marking so that oops inside diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index eaa6afb5efa..9dfdb376905 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -1932,7 +1932,7 @@ static bool should_retry_vm_op(GCCause::Cause cause, // GC, so try again. LOG_COLLECT_CONCURRENTLY(cause, "retry after in-progress"); return true; - } else if (op->whitebox_attached()) { + } else if (op->whitebox_controlled()) { // If WhiteBox wants control, wait for notification of a state // change in the controller, then try again. Don't wait for // release of control, since collections may complete while in @@ -2000,7 +2000,7 @@ bool G1CollectedHeap::try_collect_concurrently(size_t allocation_word_size, } // When _wb_breakpoint there can't be another cycle or deferred. assert(!op.cycle_already_in_progress(), "invariant"); - assert(!op.whitebox_attached(), "invariant"); + assert(!op.whitebox_controlled(), "invariant"); // Concurrent cycle attempt might have been cancelled by some other // collection, so retry. Unlike other cases below, we want to retry // even if cancelled by a STW full collection, because we really want @@ -2025,15 +2025,21 @@ bool G1CollectedHeap::try_collect_concurrently(size_t allocation_word_size, // Cases (2) and (3) are detected together by a change to // _old_marking_cycles_started. // - // Compared to other "automatic" GCs (see below), we do not consider being - // in whitebox as sufficient too because we might be anywhere within that - // cycle and we need to make progress. + // Compared to other "automatic" GCs (see below), being in WhiteBox is not + // addressed here because we need to handle it specially. if (op.mark_in_progress() || (old_marking_started_before != old_marking_started_after)) { LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, true); return true; } + if (op.whitebox_controlled()) { + LOG_COLLECT_CONCURRENTLY(cause, "Suppressed CodeCache GC because of WhiteBox in control."); + // The caller in this case does not check the return value, so it does not + // really matter what we return. However we did not finish the request. + return false; + } + if (wait_full_mark_finished(cause, old_marking_started_before, old_marking_started_after, @@ -2041,7 +2047,11 @@ bool G1CollectedHeap::try_collect_concurrently(size_t allocation_word_size, return true; } - if (should_retry_vm_op(cause, &op)) { + if (op.cycle_already_in_progress()) { + // If VMOp failed because a cycle was already in progress, it + // is now complete (we just waited). But it didn't finish this + // request, so try again. + LOG_COLLECT_CONCURRENTLY(cause, "retry after in-progress"); continue; } } else if (!GCCause::is_user_requested_gc(cause)) { @@ -2062,7 +2072,7 @@ bool G1CollectedHeap::try_collect_concurrently(size_t allocation_word_size, // _old_marking_cycles_started. if (op.gc_succeeded() || op.cycle_already_in_progress() || - op.whitebox_attached() || + op.whitebox_controlled() || (old_marking_started_before != old_marking_started_after)) { LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, true); return true; diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp index e2c01f9a13e..2414fdd7840 100644 --- a/src/hotspot/share/gc/g1/g1Policy.cpp +++ b/src/hotspot/share/gc/g1/g1Policy.cpp @@ -1253,9 +1253,9 @@ void G1Policy::update_survivors_policy() { } bool G1Policy::force_concurrent_start_if_outside_cycle(GCCause::Cause gc_cause) { - // We actually check whether we are marking here and not if we are in a - // reclamation phase. This means that we will schedule a concurrent mark - // even while we are still in the process of reclaiming memory. + // Check whether a concurrent cycle is active, do not include the + // reclamation/mixed phase. This means that we can schedule a concurrent cycle + // even while in the mixed phase. bool during_cycle = collector_state()->is_in_concurrent_cycle(); if (!during_cycle) { log_debug(gc, ergo)("Request concurrent cycle initiation (requested by GC cause). " diff --git a/src/hotspot/share/gc/g1/g1VMOperations.cpp b/src/hotspot/share/gc/g1/g1VMOperations.cpp index 373ec9660da..577f3b5491d 100644 --- a/src/hotspot/share/gc/g1/g1VMOperations.cpp +++ b/src/hotspot/share/gc/g1/g1VMOperations.cpp @@ -22,6 +22,7 @@ * */ +#include "code/codeCache.hpp" #include "gc/g1/g1CollectedHeap.inline.hpp" #include "gc/g1/g1CollectorState.inline.hpp" #include "gc/g1/g1ConcurrentMarkThread.inline.hpp" @@ -66,7 +67,7 @@ VM_G1TryInitiateConcMark::VM_G1TryInitiateConcMark(size_t allocation_word_size, _transient_failure(false), _mark_in_progress(false), _cycle_already_in_progress(false), - _whitebox_attached(false), + _whitebox_controlled(false), _gc_succeeded(false) {} @@ -88,19 +89,26 @@ void VM_G1TryInitiateConcMark::doit() { G1CollectorState* state = g1h->collector_state(); _mark_in_progress = state->is_in_marking(); _cycle_already_in_progress = state->is_in_concurrent_cycle(); + _whitebox_controlled = (_gc_cause != GCCause::_wb_breakpoint) && ConcurrentGCBreakpoints::is_controlled(); - if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) { + // Notify the code cache that we deferred clearing the unloading GC request if we are WhiteBox controlled + // and we are going to suppress it. If marking is active, we do not need to suppress because that will satisfy the + // request already. + // This needs to be atomic wrt. to all code-cache allocation threads to allow setting the request + // after WhiteBox releases control again. + bool defer_codecache_request = whitebox_controlled() && + GCCause::is_codecache_requested_gc(_gc_cause) && + !mark_in_progress(); + if (defer_codecache_request) { + CodeCache::defer_unloading_gc_request(); + return; + } else if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) { // Failure to force the next GC pause to be a concurrent start indicates // there is already a concurrent marking cycle in progress. Flags to indicate // that were already set, so return immediately. - } else if ((_gc_cause != GCCause::_wb_breakpoint) && - ConcurrentGCBreakpoints::is_controlled()) { - // WhiteBox wants to be in control of concurrent cycles, so don't try to - // start one. This check is after the force_concurrent_start_xxx so that a - // request will be remembered for a later partial collection, even though - // we've rejected this request. - _whitebox_attached = true; - } else { + return; + } else if (!whitebox_controlled()) { + // Only run a concurrent marking if not controlled by WhiteBox. g1h->do_collection_pause_at_safepoint(_word_size); _gc_succeeded = true; } diff --git a/src/hotspot/share/gc/g1/g1VMOperations.hpp b/src/hotspot/share/gc/g1/g1VMOperations.hpp index 7d56ea1916f..0c12e75eef0 100644 --- a/src/hotspot/share/gc/g1/g1VMOperations.hpp +++ b/src/hotspot/share/gc/g1/g1VMOperations.hpp @@ -48,7 +48,7 @@ class VM_G1TryInitiateConcMark : public VM_GC_Collect_Operation { bool _transient_failure; bool _mark_in_progress; bool _cycle_already_in_progress; - bool _whitebox_attached; + bool _whitebox_controlled; // The concurrent start pause may be cancelled for some reasons. Keep track of // this. bool _gc_succeeded; @@ -63,7 +63,7 @@ public: bool transient_failure() const { return _transient_failure; } bool mark_in_progress() const { return _mark_in_progress; } bool cycle_already_in_progress() const { return _cycle_already_in_progress; } - bool whitebox_attached() const { return _whitebox_attached; } + bool whitebox_controlled() const { return _whitebox_controlled; } bool gc_succeeded() const { return _gc_succeeded && VM_GC_Operation::gc_succeeded(); } }; diff --git a/src/hotspot/share/gc/shared/concurrentGCBreakpoints.cpp b/src/hotspot/share/gc/shared/concurrentGCBreakpoints.cpp index 3a974952fea..b0a784e9282 100644 --- a/src/hotspot/share/gc/shared/concurrentGCBreakpoints.cpp +++ b/src/hotspot/share/gc/shared/concurrentGCBreakpoints.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -22,6 +22,7 @@ * */ +#include "code/codeCache.hpp" #include "gc/shared/collectedHeap.hpp" #include "gc/shared/concurrentGCBreakpoints.hpp" #include "logging/log.hpp" @@ -89,6 +90,7 @@ void ConcurrentGCBreakpoints::release_control() { MonitorLocker ml(monitor()); log_trace(gc, breakpoint)("release_control"); reset_request_state(); + CodeCache::clear_deferred_unloading_gc_request(); ml.notify_all(); } diff --git a/test/hotspot/jtreg/gc/g1/TestCodeCacheWhiteBox.java b/test/hotspot/jtreg/gc/g1/TestCodeCacheWhiteBox.java new file mode 100644 index 00000000000..c47dafc3203 --- /dev/null +++ b/test/hotspot/jtreg/gc/g1/TestCodeCacheWhiteBox.java @@ -0,0 +1,216 @@ +/* + * 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.g1; + +/* + * @test TestCodeCacheWhiteBox.java + * @bug 8225186 + * @summary Test to make sure that code cache unloading does not make the VM hang when receiving + * a request while WhiteBox is holding control. + * We do that by triggering a code cache gc request (by triggering compilations) during a + * synchronous compilation while whitebox is holding control, and additionally verify that + * after the concurrent cycle additional code cache gc requests start more concurrent cycles. + * @requires vm.gc.G1 + * @requires vm.flagless + * @library /test/lib /testlibrary / + * @modules java.base/jdk.internal.misc + * java.management + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm -Xmx20M -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:. gc.g1.TestCodeCacheWhiteBox + */ + + +import java.lang.reflect.Field; + +import java.net.URL; +import java.net.URLClassLoader; + +import jdk.test.lib.Asserts; +import jdk.test.lib.Platform; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; +import jdk.test.whitebox.WhiteBox; + +public class TestCodeCacheWhiteBox { + public static final String AFTER_FIRST_CYCLE_MARKER = "Marker for this test"; + + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + private static OutputAnalyzer runTest(String concPhase) throws Exception { + OutputAnalyzer output = ProcessTools.executeLimitedTestJava("-XX:+UseG1GC", + "-Xmx20M", + "-XX:+UnlockDiagnosticVMOptions", + "-Xbootclasspath/a:.", + "-Xbatch", // Needed to make compilation synchronous + "-Xlog:gc=trace,codecache", + "-XX:+WhiteBoxAPI", + "-XX:ReservedCodeCacheSize=" + (Platform.is32bit() ? "4M" : "8M"), + "-XX:StartAggressiveSweepingAt=50", + "-XX:CompileCommand=compileonly,gc.g1.SomeClass::*", + "-XX:CompileCommand=compileonly,gc.g1.Foo*::*", + TestCodeCacheWhiteBoxRunner.class.getName(), + concPhase); + return output; + } + + private static void runAndCheckTest(String test) throws Exception { + OutputAnalyzer output; + + output = runTest(test); + output.shouldHaveExitValue(0); + output.shouldNotContain("ERROR"); + System.out.println(output.getStdout()); + + String[] parts = output.getStdout().split(AFTER_FIRST_CYCLE_MARKER); + + // Either "Threshold" or "Aggressive" CodeCache GC are fine for the test. + final String codecacheGCStart = "Pause Young (Concurrent Start) (CodeCache GC "; + + boolean success = parts.length == 2 && parts[1].indexOf(codecacheGCStart) != -1; + Asserts.assertTrue(success, "Could not find a CodeCache GC Threshold GC after finishing the concurrent cycle"); + } + + public static void main(String[] args) throws Exception { + runAndCheckTest(WB.BEFORE_MARKING_COMPLETED); // This one should always complete. Just for sanity checking. + runAndCheckTest(WB.G1_BEFORE_REBUILD_COMPLETED); + runAndCheckTest(WB.G1_BEFORE_CLEANUP_COMPLETED); + } +} + +class TestCodeCacheWhiteBoxRunner { + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + private static void refClass(Class clazz) throws Exception { + Field name = clazz.getDeclaredField("NAME"); + name.setAccessible(true); + name.get(null); + } + + private static class MyClassLoader extends URLClassLoader { + public MyClassLoader(URL url) { + super(new URL[]{url}, null); + } + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + try { + return super.loadClass(name, resolve); + } catch (ClassNotFoundException e) { + return Class.forName(name, resolve, TestCodeCacheWhiteBoxRunner.class.getClassLoader()); + } + } + } + + private static void triggerCodeCacheGC() { + URL url = TestCodeCacheWhiteBoxRunner.class.getProtectionDomain().getCodeSource().getLocation(); + try { + int i = 0; + do { + ClassLoader cl = new MyClassLoader(url); + refClass(cl.loadClass("gc.g1.SomeClass")); + + if (i % 20 == 0) { + System.out.println("Compiled " + i + " classes"); + } + i++; + } while (i < 200); + System.out.println("Compilation done, compiled " + i + " classes"); + } catch (Throwable t) { + System.out.println("ERROR: threw exception " + t); + } + } + + public static void main(String[] args) throws Exception { + System.out.println("Running to breakpoint: " + args[0]); + try { + WB.concurrentGCAcquireControl(); + WB.concurrentGCRunTo(args[0]); + + System.out.println("Try to trigger code cache GC"); + + Thread toRun = new Thread(() -> + { + System.out.println("Thread is running"); + triggerCodeCacheGC(); + System.out.println("Thread completed"); + }); + toRun.setDaemon(true); // non-daemon thread could prevent VM shutdown after the main thread times out + toRun.start(); + toRun.join(60_000); + + if (toRun.isAlive()) { + toRun.interrupt(); + throw new RuntimeException("ERROR: thread took too long, deadlocked?"); + } + + WB.concurrentGCRunToIdle(); + } catch (InterruptedException e) { + System.out.println("ERROR: starting helper thread"); + throw e; + } finally { + // Make sure that the marker we use to find the expected log message is printed + // before we release whitebox control, i.e. before the expected garbage collection + // can start. + System.out.println(TestCodeCacheWhiteBox.AFTER_FIRST_CYCLE_MARKER); + WB.concurrentGCReleaseControl(); + } + Thread.sleep(1000); + triggerCodeCacheGC(); + } +} + +abstract class Foo { + public abstract int foo(); +} + +class Foo1 extends Foo { + private int a; + public int foo() { return a; } +} + +class Foo2 extends Foo { + private int a; + public int foo() { return a; } +} + +class Foo3 extends Foo { + private int a; + public int foo() { return a; } +} + +class Foo4 extends Foo { + private int a; + public int foo() { return a; } +} + +class SomeClass { + static final String NAME = "name"; + + static { + int res =0; + Foo[] foos = new Foo[] { new Foo1(), new Foo2(), new Foo3(), new Foo4() }; + for (int i = 0; i < 100000; i++) { + res = foos[i % foos.length].foo(); + } + } +}