From 0fd6bc7dcec24d9bcf5c22f53019870afd0b8f2d Mon Sep 17 00:00:00 2001 From: Anton Artemov Date: Mon, 19 Jan 2026 12:14:13 +0100 Subject: [PATCH] 8366659: Addressed reviewers' comments, added comments, renamed tests. --- src/hotspot/share/runtime/objectMonitor.cpp | 40 +++++++++++++------ .../SuspendWithObjectMonitorWaitBase.java | 6 +-- ... SuspendWithObjectMonitorWaitDefault.java} | 11 ++--- ...ithObjectMonitorWaitReentryPartFirst.java} | 12 +++--- ...thObjectMonitorWaitReentryPartSecond.java} | 12 +++--- 5 files changed, 49 insertions(+), 32 deletions(-) rename test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/{SuspendWithObjectMonitorWait1.java => SuspendWithObjectMonitorWaitDefault.java} (94%) rename test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/{SuspendWithObjectMonitorWait2.java => SuspendWithObjectMonitorWaitReentryPartFirst.java} (93%) rename test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/{SuspendWithObjectMonitorWait3.java => SuspendWithObjectMonitorWaitReentryPartSecond.java} (93%) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index e1dafc8f1bb..506895cdcc8 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -1866,7 +1866,7 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { // while (!timeout && !interrupted && node.TState == TS_WAIT) park() int ret = OS_OK; - bool was_notified = false; + bool was_notified = true; // Need to check interrupt state whilst still _thread_in_vm bool interrupted = interruptible && current->is_interrupted(false); @@ -1880,7 +1880,7 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { { ThreadBlockInVM tbivm(current, false /* allow_suspend */); if (interrupted || HAS_PENDING_EXCEPTION) { - // Intentionally empty + was_notified = false; } else if (node.TState == ObjectWaiter::TS_WAIT) { if (millis <= 0) { current->_ParkEvent->park(); @@ -1905,7 +1905,6 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { // then we'll acquire the lock and then re-fetch a fresh TState value. // That is, we fail toward safety. - was_notified = true; if (node.TState == ObjectWaiter::TS_WAIT) { SpinCriticalSection scs(&_wait_set_lock); if (node.TState == ObjectWaiter::TS_WAIT) { @@ -1914,7 +1913,6 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { was_notified = false; } } - // The thread is now either on off-list (TS_RUN), // or on the entry_list (TS_ENTER). // The Node's TState variable is stable from the perspective of this thread. @@ -1930,7 +1928,7 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { // although the raw address of the object may have changed. // (Don't cache naked oops over safepoints, of course). - // post monitor waited event. Note that this is past-tense, we are done waiting. + // Post monitor waited event. Note that this is past-tense, we are done waiting. if (JvmtiExport::should_post_monitor_waited() && node.TState != ObjectWaiter::TS_ENTER) { // Process suspend requests now if any, before posting the event. @@ -1972,7 +1970,6 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { // and set the OM as pending, the thread will not be reported as // having "-locked" the monitor. } - // Done waiting, post the corresponding event if (eos.exited()) { // ExitOnSuspend exit the OM assert(!has_owner(current), "invariant"); @@ -2053,6 +2050,9 @@ bool ObjectMonitor::notify_internal(JavaThread* current) { // Adding to _entry_list uses Atomic::cmpxchg() which already provides // a fence that prevents reordering of the stores. if (!JvmtiExport::should_post_monitor_waited()) { + // This is is now conditional as if the monitor_waited even + // is allowed, then a thread, even virtual, should not be moved to + // the entry_list, but rather unparked and let run. See the comment below. inc_unmounted_vthreads(); } } @@ -2061,14 +2061,27 @@ bool ObjectMonitor::notify_internal(JavaThread* current) { did_notify = true; if (!JvmtiExport::should_post_monitor_waited()) { + // If the monitor_waited JVMTI event is not allowed, a thread is + // transferred to the entry_list, and it will eventually be unparked + // only when it is chosen to become a successor. add_to_entry_list(current, iterator); } else { + // However, if the monitor_waited event is allowed, then + // a thread is unpraked immediately and let run. It is not + // on the entry_list, and thus cannot become a successor. + // Its state should be TS_RUN, as it is neither on wait_list + // nor on the entry_list. + // By not allowing a thread to become a successor we + // avoid a problem of having a suspension point when posting + // the monitor_waited JVMTI event, as suspending such a thread + // is no harm. + iterator->TState = ObjectWaiter::TS_RUN; OrderAccess::fence(); oop vthread = nullptr; - ParkEvent* Trigger; + ParkEvent* evt; if (!iterator->is_vthread()) { iterator->wait_reenter_begin(this); if (has_unmounted_vthreads()) { @@ -2076,22 +2089,21 @@ bool ObjectMonitor::notify_internal(JavaThread* current) { } JavaThread* t = iterator->thread(); assert(t != nullptr, ""); - Trigger = t->_ParkEvent; + evt = t->_ParkEvent; } else { vthread = iterator->vthread(); assert(vthread != nullptr, ""); - Trigger = ObjectMonitor::vthread_unparker_ParkEvent(); + evt = ObjectMonitor::vthread_unparker_ParkEvent(); } if (vthread == nullptr) { // Platform thread case. - Trigger->unpark(); + evt->unpark(); } else if (java_lang_VirtualThread::set_onWaitingList(vthread, vthread_list_head())) { // Virtual thread case. - Trigger->unpark(); + evt->unpark(); } - return did_notify; } @@ -2276,6 +2288,10 @@ bool ObjectMonitor::vthread_wait_reenter(JavaThread* current, ObjectWaiter* node // Mark that we are at reenter so that we don't call this method again. node->_at_reenter = true; + // We check against the state, rather than if was_notified, + // because, if monitor_waited JVMTI event is allowed, there can be a vthead which + // is not on the entry_list, but has been notified in the sense that it has been + // unparked directly in notify_internal(). Its state is then TS_RUN. if (state == ObjectWaiter::TS_RUN) { bool acquired = vthread_monitor_enter(current, node); if (acquired) { diff --git a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitBase.java b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitBase.java index 4d1377ab088..5443e4005fe 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitBase.java +++ b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitBase.java @@ -218,13 +218,13 @@ public class SuspendWithObjectMonitorWaitBase { SuspendWithObjectMonitorWaitBase test = null; switch (testCase) { case 1: - test = new SuspendWithObjectMonitorWait1(); + test = new SuspendWithObjectMonitorWaitDefault(); break; case 2: - test = new SuspendWithObjectMonitorWait2(); + test = new SuspendWithObjectMonitorWaitReentryPartFirst(); break; case 3: - test = new SuspendWithObjectMonitorWait3(); + test = new SuspendWithObjectMonitorWaitReentryPartSecond(); break; default: // Impossible diff --git a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait1.java b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitDefault.java similarity index 94% rename from test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait1.java rename to test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitDefault.java index 921afbd402c..b2c1f108de5 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait1.java +++ b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitDefault.java @@ -27,14 +27,15 @@ * @summary Test SuspendThread with ObjectMonitor wait. * @requires vm.jvmti * @library /test/lib - * @compile SuspendWithObjectMonitorWait1.java - * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWait1 1 + * @compile SuspendWithObjectMonitorWaitDefault.java + * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWaitDefault 1 */ import java.io.PrintStream; // -// doWork1 algorithm: +// SuspendWithObjectMonitorWaitDefault algorithm: +// // main waiter resumer // ================= ================== =================== // launch waiter @@ -58,7 +59,7 @@ import java.io.PrintStream; // waiter exits // -public class SuspendWithObjectMonitorWait1 extends SuspendWithObjectMonitorWaitBase { +public class SuspendWithObjectMonitorWaitDefault extends SuspendWithObjectMonitorWaitBase { @Override public int run(int timeMax, PrintStream out) { @@ -68,7 +69,7 @@ public class SuspendWithObjectMonitorWait1 extends SuspendWithObjectMonitorWaitB // Default scenario, the resumer thread is always able to grab the threadLock once notified by the main thread. public int doWork1(int timeMax, PrintStream out) { SuspendWithObjectMonitorWaitWorker waiter; // waiter thread - SuspendWithObjectMonitorWaitWorker resumer; // resumer thread + SuspendWithObjectMonitorWaitWorker resumer; // resumer thread System.out.println("Test 1: About to execute for " + timeMax + " seconds."); diff --git a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait2.java b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartFirst.java similarity index 93% rename from test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait2.java rename to test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartFirst.java index 9ba39e742b0..b331b338f47 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait2.java +++ b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartFirst.java @@ -27,14 +27,14 @@ * @summary Test SuspendThread with ObjectMonitor wait. * @requires vm.jvmti * @library /test/lib - * @compile SuspendWithObjectMonitorWait2.java - * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWait2 2 + * @compile SuspendWithObjectMonitorWaitReentryPartFirst.java + * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWaitReentryPartFirst 2 */ import java.io.PrintStream; // -// doWork2 algorithm: +// SuspendWithObjectMonitorWaitReentryPartFirst algorithm: // // main waiter resumer // ================= ================== =================== @@ -61,13 +61,13 @@ import java.io.PrintStream; // // Note: The sleep(1-second) in main along with the delayed exit // of threadLock in main forces the resumer thread to reach -// "enter threadLock" and block. This difference from doWork1 +// "enter threadLock" and block. This difference from the default scenario // forces the resumer thread to be contending for threadLock // while the waiter thread is in threadLock.wait() increasing // stress on the monitor sub-system. // -public class SuspendWithObjectMonitorWait2 extends SuspendWithObjectMonitorWaitBase { +public class SuspendWithObjectMonitorWaitReentryPartFirst extends SuspendWithObjectMonitorWaitBase { @Override public int run(int timeMax, PrintStream out) { @@ -77,7 +77,7 @@ public class SuspendWithObjectMonitorWait2 extends SuspendWithObjectMonitorWaitB // Notify the resumer while holding the threadLock. public int doWork2(int timeMax, PrintStream out) { SuspendWithObjectMonitorWaitWorker waiter; // waiter thread - SuspendWithObjectMonitorWaitWorker resumer; // resumer thread + SuspendWithObjectMonitorWaitWorker resumer; // resumer thread System.out.println("Test 2: About to execute for " + timeMax + " seconds."); diff --git a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait3.java b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartSecond.java similarity index 93% rename from test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait3.java rename to test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartSecond.java index ed3070d4a5b..92ab5a88eb6 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait3.java +++ b/test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWaitReentryPartSecond.java @@ -27,14 +27,14 @@ * @summary Test SuspendThread with ObjectMonitor wait. * @requires vm.jvmti * @library /test/lib - * @compile SuspendWithObjectMonitorWait3.java - * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWait3 3 + * @compile SuspendWithObjectMonitorWaitReentryPartSecond.java + * @run main/othervm/native -agentlib:SuspendWithObjectMonitorWait SuspendWithObjectMonitorWaitReentryPartSecond 3 */ import java.io.PrintStream; // -// doWork3 algorithm: +// SuspendWithObjectMonitorWaitReentryPartSecond algorithm: // // main waiter resumer // =================== ====================== =================== @@ -63,7 +63,7 @@ import java.io.PrintStream; // // Note: The sleep(1-second) in main along with the delayed exit // of threadLock in main forces the resumer thread to reach -// "enter threadLock" and block. This difference from doWork1 +// "enter threadLock" and block. This difference from the default scenario // forces the resumer thread to be contending for threadLock // while the waiter thread is in the threadLock.wait(1) tight // loop increasing stress on the monitor sub-system. @@ -73,7 +73,7 @@ import java.io.PrintStream; // suspend the waiter thread. // -public class SuspendWithObjectMonitorWait3 extends SuspendWithObjectMonitorWaitBase { +public class SuspendWithObjectMonitorWaitReentryPartSecond extends SuspendWithObjectMonitorWaitBase { @Override public int run(int timeMax, PrintStream out) { @@ -83,7 +83,7 @@ public class SuspendWithObjectMonitorWait3 extends SuspendWithObjectMonitorWaitB // Suspend on the re-entry path of wait. public int doWork3(int timeMax, PrintStream out) { SuspendWithObjectMonitorWaitWorker waiter; // waiter thread - SuspendWithObjectMonitorWaitWorker resumer; // resumer thread + SuspendWithObjectMonitorWaitWorker resumer; // resumer thread System.out.println("Test 3: About to execute for " + timeMax + " seconds.");