From c6a7325f114ed48b120add189f8289d729a85d63 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 19 Mar 2025 16:23:48 +0100 Subject: [PATCH 1/3] Blind fix --- src/hotspot/share/runtime/objectMonitor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 510b5cca5f445..ab1a8e804f041 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -737,6 +737,7 @@ bool ObjectMonitor::try_lock_or_add_to_entry_list(JavaThread* current, ObjectWai static void post_monitor_deflate_event(EventJavaMonitorDeflate* event, const oop obj) { assert(event != nullptr, "invariant"); + assert(obj != nullptr, "invariant"); const Klass* monitor_klass = obj->klass(); if (ObjectMonitor::is_jfr_excluded(monitor_klass)) { return; @@ -840,7 +841,7 @@ bool ObjectMonitor::deflate_monitor(Thread* current) { install_displaced_markword_in_object(obj); } - if (event.should_commit()) { + if (obj != nullptr && event.should_commit()) { post_monitor_deflate_event(&event, obj); } From 2b1d1b14faf7444bcf2f7fcc87e32904453c781c Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 19 Mar 2025 18:36:40 +0100 Subject: [PATCH 2/3] Stress test --- .../runtime/StressJavaMonitorEvents.java | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 test/jdk/jdk/jfr/event/runtime/StressJavaMonitorEvents.java diff --git a/test/jdk/jdk/jfr/event/runtime/StressJavaMonitorEvents.java b/test/jdk/jdk/jfr/event/runtime/StressJavaMonitorEvents.java new file mode 100644 index 0000000000000..ed3b5454163ab --- /dev/null +++ b/test/jdk/jdk/jfr/event/runtime/StressJavaMonitorEvents.java @@ -0,0 +1,156 @@ +/* + * Copyright Amazon.com Inc. 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 jdk.jfr.event.runtime; + +import static jdk.test.lib.Asserts.assertTrue; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.ThreadLocalRandom; + +import jdk.jfr.consumer.RecordingStream; +import jdk.jfr.consumer.RecordedClass; +import jdk.jfr.consumer.RecordedEvent; +import jdk.test.lib.jfr.EventNames; +import jdk.test.lib.jfr.Events; +import jdk.test.lib.thread.TestThread; +import jdk.test.lib.thread.XRun; + +/** + * @test StressJavaMonitorEvents + * @summary Tests that VM does not crash when monitor-related JFR events are enabled + * @requires vm.flagless + * @requires vm.hasJFR + * @library /test/lib + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:GuaranteedAsyncDeflationInterval=100 jdk.jfr.event.runtime.StressJavaMonitorEvents + */ +public class StressJavaMonitorEvents { + + static final int RUN_TIME_MS = 10000; + static final int GC_EVERY_MS = 500; + static final int THREADS = 4; + static final int NUM_LOCKS = 1024; + + static final List CAPTURE_EVENTS = List.of( + EventNames.JavaMonitorEnter, + EventNames.JavaMonitorWait, + EventNames.JavaMonitorNotify, + EventNames.JavaMonitorInflate, + EventNames.JavaMonitorDeflate, + EventNames.JavaMonitorStatistics + ); + + static final Set CAN_BE_ZERO_EVENTS = Set.of( + // Only when lock actually gets contended, not guaranteed. + EventNames.JavaMonitorEnter, + // Only when there are waiters on the lock, not guaranteed. + EventNames.JavaMonitorNotify + ); + + static final Object[] LOCKS = new Object[NUM_LOCKS]; + + public static TestThread startThread(final long deadline) { + TestThread t = new TestThread(new XRun() { + @Override + public void xrun() throws Throwable { + ThreadLocalRandom r = ThreadLocalRandom.current(); + while (System.nanoTime() < deadline) { + // Overwrite random lock, making the old one dead + LOCKS[r.nextInt(NUM_LOCKS)] = new Object(); + + // Wait on random lock, inflating it + Object waitLock = LOCKS[r.nextInt(NUM_LOCKS)]; + if (waitLock != null) { + synchronized (waitLock) { + waitLock.wait(1); + } + } + + // Notify a random lock + Object notifyLock = LOCKS[r.nextInt(NUM_LOCKS)]; + if (notifyLock != null) { + synchronized (notifyLock) { + notifyLock.notify(); + } + } + + // Notify all on a random lock + Object notifyAllLock = LOCKS[r.nextInt(NUM_LOCKS)]; + if (notifyAllLock != null) { + synchronized (notifyAllLock) { + notifyAllLock.notifyAll(); + } + } + } + } + }); + t.start(); + return t; + } + + public static void main(String[] args) throws Exception { + Map counters = new HashMap<>(); + + try (RecordingStream rs = new RecordingStream()) { + // Setup all interesting events, and start recording. + for (String ev : CAPTURE_EVENTS) { + rs.enable(ev).withoutThreshold(); + AtomicLong counter = new AtomicLong(); + rs.onEvent(ev, e -> counter.incrementAndGet()); + counters.put(ev, counter); + } + rs.startAsync(); + + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(RUN_TIME_MS); + List threads = new ArrayList<>(); + for (int t = 0; t < THREADS; t++) { + threads.add(startThread(deadline)); + } + + // Trigger GCs periodically to yield dead objects with inflated monitors. + while (System.nanoTime() < deadline) { + Thread.sleep(GC_EVERY_MS); + System.gc(); + } + + // Wait for all threads to exit and close the recording. + for (TestThread t : threads) { + t.join(); + } + rs.close(); + + // Print stats and check event counts. + for (String ev : CAPTURE_EVENTS) { + long count = counters.get(ev).get(); + System.out.println(ev + ": " + count); + assertTrue(CAN_BE_ZERO_EVENTS.contains(ev) || (count > 0)); + } + } + } +} From 862a01766f80a0940602b05ffa1c7982eece9977 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Thu, 20 Mar 2025 08:02:13 +0100 Subject: [PATCH 3/3] Emit the event anyway --- src/hotspot/share/jfr/metadata/metadata.xml | 4 ++-- src/hotspot/share/runtime/objectMonitor.cpp | 20 ++++++++++++------- .../runtime/TestJavaMonitorDeflateEvent.java | 5 ++++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/hotspot/share/jfr/metadata/metadata.xml b/src/hotspot/share/jfr/metadata/metadata.xml index f430c43eb6258..562d31b828d3e 100644 --- a/src/hotspot/share/jfr/metadata/metadata.xml +++ b/src/hotspot/share/jfr/metadata/metadata.xml @@ -123,8 +123,8 @@ - - + + diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index ab1a8e804f041..c36ddee1d201d 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -737,13 +737,19 @@ bool ObjectMonitor::try_lock_or_add_to_entry_list(JavaThread* current, ObjectWai static void post_monitor_deflate_event(EventJavaMonitorDeflate* event, const oop obj) { assert(event != nullptr, "invariant"); - assert(obj != nullptr, "invariant"); - const Klass* monitor_klass = obj->klass(); - if (ObjectMonitor::is_jfr_excluded(monitor_klass)) { - return; + if (obj == nullptr) { + // Accept the case when obj was already garbage-collected. + // Emit the event anyway, but without details. + event->set_monitorClass(nullptr); + event->set_address(0); + } else { + const Klass* monitor_klass = obj->klass(); + if (ObjectMonitor::is_jfr_excluded(monitor_klass)) { + return; + } + event->set_monitorClass(monitor_klass); + event->set_address((uintptr_t)(void*)obj); } - event->set_monitorClass(monitor_klass); - event->set_address((uintptr_t)(void*)obj); event->commit(); } @@ -841,7 +847,7 @@ bool ObjectMonitor::deflate_monitor(Thread* current) { install_displaced_markword_in_object(obj); } - if (obj != nullptr && event.should_commit()) { + if (event.should_commit()) { post_monitor_deflate_event(&event, obj); } diff --git a/test/jdk/jdk/jfr/event/runtime/TestJavaMonitorDeflateEvent.java b/test/jdk/jdk/jfr/event/runtime/TestJavaMonitorDeflateEvent.java index 2c10911179b3d..5e1e604a50434 100644 --- a/test/jdk/jdk/jfr/event/runtime/TestJavaMonitorDeflateEvent.java +++ b/test/jdk/jdk/jfr/event/runtime/TestJavaMonitorDeflateEvent.java @@ -56,8 +56,11 @@ public class TestJavaMonitorDeflateEvent { static class Lock { } + // Make sure the object stays reachable. + // This guarantees the fields are fully set up on deflation. + static final Lock lock = new Lock(); + public static void main(String[] args) throws Exception { - final Lock lock = new Lock(); final String lockClassName = lock.getClass().getName().replace('.', '/'); List events = new CopyOnWriteArrayList<>();