# HG changeset patch # User dholmes # Date 1570488293 14400 # Node ID 165b193b30dd87fe41f5a967fd0bf25f0c405657 # Parent 43f63f904bbc3805343a417a2e34bb44b383da23 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up Reviewed-by: sspitsyn, dcubed, coleenp diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/prims/jvmtiEnv.cpp --- a/src/hotspot/share/prims/jvmtiEnv.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/prims/jvmtiEnv.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -3229,23 +3229,23 @@ jvmtiError JvmtiEnv::DestroyRawMonitor(JvmtiRawMonitor * rmonitor) { if (Threads::number_of_threads() == 0) { - // Remove this monitor from pending raw monitors list + // Remove this monitor from pending raw monitors list // if it has entered in onload or start phase. JvmtiPendingMonitors::destroy(rmonitor); } else { Thread* thread = Thread::current(); - if (rmonitor->is_entered(thread)) { + if (rmonitor->owner() == thread) { // The caller owns this monitor which we are about to destroy. // We exit the underlying synchronization object so that the // "delete monitor" call below can work without an assertion // failure on systems that don't like destroying synchronization // objects that are locked. int r; - intptr_t recursion = rmonitor->recursions(); - for (intptr_t i = 0; i <= recursion; i++) { + int recursion = rmonitor->recursions(); + for (int i = 0; i <= recursion; i++) { r = rmonitor->raw_exit(thread); - assert(r == ObjectMonitor::OM_OK, "raw_exit should have worked"); - if (r != ObjectMonitor::OM_OK) { // robustness + assert(r == JvmtiRawMonitor::M_OK, "raw_exit should have worked"); + if (r != JvmtiRawMonitor::M_OK) { // robustness return JVMTI_ERROR_INTERNAL; } } @@ -3271,7 +3271,7 @@ jvmtiError JvmtiEnv::RawMonitorEnter(JvmtiRawMonitor * rmonitor) { if (Threads::number_of_threads() == 0) { - // No JavaThreads exist so ObjectMonitor enter cannot be + // No JavaThreads exist so JvmtiRawMonitor enter cannot be // used, add this raw monitor to the pending list. // The pending monitors will be actually entered when // the VM is setup. @@ -3279,20 +3279,10 @@ // in thread.cpp. JvmtiPendingMonitors::enter(rmonitor); } else { - int r = 0; Thread* thread = Thread::current(); - if (thread->is_Java_thread()) { JavaThread* current_thread = (JavaThread*)thread; -#ifdef PROPER_TRANSITIONS - // Not really unknown but ThreadInVMfromNative does more than we want - ThreadInVMfromUnknown __tiv; - { - ThreadBlockInVM __tbivm(current_thread); - r = rmonitor->raw_enter(current_thread); - } -#else /* Transition to thread_blocked without entering vm state */ /* This is really evil. Normally you can't undo _thread_blocked */ /* transitions like this because it would cause us to miss a */ @@ -3308,22 +3298,11 @@ current_thread->frame_anchor()->walkable(), "Must be walkable"); current_thread->set_thread_state(_thread_blocked); - r = rmonitor->raw_enter(current_thread); + rmonitor->raw_enter(current_thread); // restore state, still at a safepoint safe state current_thread->set_thread_state(state); - -#endif /* PROPER_TRANSITIONS */ - assert(r == ObjectMonitor::OM_OK, "raw_enter should have worked"); } else { - if (thread->is_Named_thread()) { - r = rmonitor->raw_enter(thread); - } else { - ShouldNotReachHere(); - } - } - - if (r != ObjectMonitor::OM_OK) { // robustness - return JVMTI_ERROR_INTERNAL; + rmonitor->raw_enter(thread); } } return JVMTI_ERROR_NONE; @@ -3342,31 +3321,10 @@ err = JVMTI_ERROR_NOT_MONITOR_OWNER; } } else { - int r = 0; Thread* thread = Thread::current(); - - if (thread->is_Java_thread()) { - JavaThread* current_thread = (JavaThread*)thread; -#ifdef PROPER_TRANSITIONS - // Not really unknown but ThreadInVMfromNative does more than we want - ThreadInVMfromUnknown __tiv; -#endif /* PROPER_TRANSITIONS */ - r = rmonitor->raw_exit(current_thread); - } else { - if (thread->is_Named_thread()) { - r = rmonitor->raw_exit(thread); - } else { - ShouldNotReachHere(); - } - } - - if (r == ObjectMonitor::OM_ILLEGAL_MONITOR_STATE) { + int r = rmonitor->raw_exit(thread); + if (r == JvmtiRawMonitor::M_ILLEGAL_MONITOR_STATE) { err = JVMTI_ERROR_NOT_MONITOR_OWNER; - } else { - assert(r == ObjectMonitor::OM_OK, "raw_exit should have worked"); - if (r != ObjectMonitor::OM_OK) { // robustness - err = JVMTI_ERROR_INTERNAL; - } } } return err; @@ -3381,14 +3339,7 @@ if (thread->is_Java_thread()) { JavaThread* current_thread = (JavaThread*)thread; -#ifdef PROPER_TRANSITIONS - // Not really unknown but ThreadInVMfromNative does more than we want - ThreadInVMfromUnknown __tiv; - { - ThreadBlockInVM __tbivm(current_thread); - r = rmonitor->raw_wait(millis, true, current_thread); - } -#else + /* Transition to thread_blocked without entering vm state */ /* This is really evil. Normally you can't undo _thread_blocked */ /* transitions like this because it would cause us to miss a */ @@ -3408,57 +3359,31 @@ // restore state, still at a safepoint safe state current_thread->set_thread_state(state); -#endif /* PROPER_TRANSITIONS */ } else { - if (thread->is_Named_thread()) { r = rmonitor->raw_wait(millis, false, thread); - } else { - ShouldNotReachHere(); - } + assert(r != JvmtiRawMonitor::M_INTERRUPTED, "non-JavaThread can't be interrupted"); } switch (r) { - case ObjectMonitor::OM_INTERRUPTED: + case JvmtiRawMonitor::M_INTERRUPTED: return JVMTI_ERROR_INTERRUPT; - case ObjectMonitor::OM_ILLEGAL_MONITOR_STATE: + case JvmtiRawMonitor::M_ILLEGAL_MONITOR_STATE: return JVMTI_ERROR_NOT_MONITOR_OWNER; + default: + return JVMTI_ERROR_NONE; } - assert(r == ObjectMonitor::OM_OK, "raw_wait should have worked"); - if (r != ObjectMonitor::OM_OK) { // robustness - return JVMTI_ERROR_INTERNAL; - } - - return JVMTI_ERROR_NONE; } /* end RawMonitorWait */ // rmonitor - pre-checked for validity jvmtiError JvmtiEnv::RawMonitorNotify(JvmtiRawMonitor * rmonitor) { - int r = 0; Thread* thread = Thread::current(); - - if (thread->is_Java_thread()) { - JavaThread* current_thread = (JavaThread*)thread; - // Not really unknown but ThreadInVMfromNative does more than we want - ThreadInVMfromUnknown __tiv; - r = rmonitor->raw_notify(current_thread); - } else { - if (thread->is_Named_thread()) { - r = rmonitor->raw_notify(thread); - } else { - ShouldNotReachHere(); - } - } - - if (r == ObjectMonitor::OM_ILLEGAL_MONITOR_STATE) { + int r = rmonitor->raw_notify(thread); + + if (r == JvmtiRawMonitor::M_ILLEGAL_MONITOR_STATE) { return JVMTI_ERROR_NOT_MONITOR_OWNER; } - assert(r == ObjectMonitor::OM_OK, "raw_notify should have worked"); - if (r != ObjectMonitor::OM_OK) { // robustness - return JVMTI_ERROR_INTERNAL; - } - return JVMTI_ERROR_NONE; } /* end RawMonitorNotify */ @@ -3466,29 +3391,12 @@ // rmonitor - pre-checked for validity jvmtiError JvmtiEnv::RawMonitorNotifyAll(JvmtiRawMonitor * rmonitor) { - int r = 0; Thread* thread = Thread::current(); - - if (thread->is_Java_thread()) { - JavaThread* current_thread = (JavaThread*)thread; - ThreadInVMfromUnknown __tiv; - r = rmonitor->raw_notifyAll(current_thread); - } else { - if (thread->is_Named_thread()) { - r = rmonitor->raw_notifyAll(thread); - } else { - ShouldNotReachHere(); - } - } - - if (r == ObjectMonitor::OM_ILLEGAL_MONITOR_STATE) { + int r = rmonitor->raw_notifyAll(thread); + + if (r == JvmtiRawMonitor::M_ILLEGAL_MONITOR_STATE) { return JVMTI_ERROR_NOT_MONITOR_OWNER; } - assert(r == ObjectMonitor::OM_OK, "raw_notifyAll should have worked"); - if (r != ObjectMonitor::OM_OK) { // robustness - return JVMTI_ERROR_INTERNAL; - } - return JVMTI_ERROR_NONE; } /* end RawMonitorNotifyAll */ diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/prims/jvmtiEnvBase.cpp --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -659,10 +659,9 @@ // thread is not doing an Object.wait() call mon = java_thread->current_pending_monitor(); if (mon != NULL) { - // The thread is trying to enter() or raw_enter() an ObjectMonitor. + // The thread is trying to enter() an ObjectMonitor. obj = (oop)mon->object(); - // If obj == NULL, then ObjectMonitor is raw which doesn't count - // as contended for this API + assert(obj != NULL, "ObjectMonitor should have a valid object!"); } // implied else: no contended ObjectMonitor } else { diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/prims/jvmtiRawMonitor.cpp --- a/src/hotspot/share/prims/jvmtiRawMonitor.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/prims/jvmtiRawMonitor.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -30,21 +30,23 @@ #include "runtime/orderAccess.hpp" #include "runtime/thread.inline.hpp" -GrowableArray *JvmtiPendingMonitors::_monitors = new (ResourceObj::C_HEAP, mtInternal) GrowableArray(1,true); +JvmtiRawMonitor::QNode::QNode(Thread* thread) : _next(NULL), _prev(NULL), + _event(thread->_ParkEvent), + _notified(0), TState(TS_RUN) { +} + +GrowableArray *JvmtiPendingMonitors::_monitors = + new (ResourceObj::C_HEAP, mtInternal) GrowableArray(1, true); void JvmtiPendingMonitors::transition_raw_monitors() { assert((Threads::number_of_threads()==1), - "Java thread has not created yet or more than one java thread \ + "Java thread has not been created yet or more than one java thread \ is running. Raw monitor transition will not work"); JavaThread *current_java_thread = JavaThread::current(); assert(current_java_thread->thread_state() == _thread_in_vm, "Must be in vm"); - { - ThreadBlockInVM __tbivm(current_java_thread); - for(int i=0; i< count(); i++) { - JvmtiRawMonitor *rmonitor = monitors()->at(i); - int r = rmonitor->raw_enter(current_java_thread); - assert(r == ObjectMonitor::OM_OK, "raw_enter should have worked"); - } + for(int i=0; i< count(); i++) { + JvmtiRawMonitor *rmonitor = monitors()->at(i); + rmonitor->raw_enter(current_java_thread); } // pending monitors are converted to real monitor so delete them all. dispose(); @@ -54,13 +56,16 @@ // class JvmtiRawMonitor // -JvmtiRawMonitor::JvmtiRawMonitor(const char *name) { +JvmtiRawMonitor::JvmtiRawMonitor(const char *name) : _owner(NULL), + _recursions(0), + _EntryList(NULL), + _WaitSet(NULL), + _waiters(0), + _magic(JVMTI_RM_MAGIC), + _name(NULL) { #ifdef ASSERT _name = strcpy(NEW_C_HEAP_ARRAY(char, strlen(name) + 1, mtInternal), name); -#else - _name = NULL; #endif - _magic = JVMTI_RM_MAGIC; } JvmtiRawMonitor::~JvmtiRawMonitor() { @@ -100,41 +105,29 @@ } // ------------------------------------------------------------------------- -// The raw monitor subsystem is entirely distinct from normal -// java-synchronization or jni-synchronization. raw monitors are not +// The JVMTI raw monitor subsystem is entirely distinct from normal +// java-synchronization or jni-synchronization. JVMTI raw monitors are not // associated with objects. They can be implemented in any manner // that makes sense. The original implementors decided to piggy-back -// the raw-monitor implementation on the existing Java objectMonitor mechanism. -// This flaw needs to fixed. We should reimplement raw monitors as sui-generis. -// Specifically, we should not implement raw monitors via java monitors. -// Time permitting, we should disentangle and deconvolve the two implementations -// and move the resulting raw monitor implementation over to the JVMTI directories. -// Ideally, the raw monitor implementation would be built on top of -// park-unpark and nothing else. -// -// raw monitors are used mainly by JVMTI -// The raw monitor implementation borrows the ObjectMonitor structure, -// but the operators are degenerate and extremely simple. -// -// Mixed use of a single objectMonitor instance -- as both a raw monitor -// and a normal java monitor -- is not permissible. +// the raw-monitor implementation on the existing Java ObjectMonitor mechanism. +// Now we just use a simplified form of that ObjectMonitor code. // // Note that we use the single RawMonitor_lock to protect queue operations for // _all_ raw monitors. This is a scalability impediment, but since raw monitor usage -// is deprecated and rare, this is not of concern. The RawMonitor_lock can not +// is fairly rare, this is not of concern. The RawMonitor_lock can not // be held indefinitely. The critical sections must be short and bounded. // // ------------------------------------------------------------------------- -int JvmtiRawMonitor::SimpleEnter (Thread * Self) { +void JvmtiRawMonitor::SimpleEnter (Thread * Self) { for (;;) { if (Atomic::replace_if_null(Self, &_owner)) { - return OS_OK ; + return ; } - ObjectWaiter Node (Self) ; + QNode Node (Self) ; Self->_ParkEvent->reset() ; // strictly optional - Node.TState = ObjectWaiter::TS_ENTER ; + Node.TState = QNode::TS_ENTER ; RawMonitor_lock->lock_without_safepoint_check() ; Node._next = _EntryList ; @@ -143,21 +136,21 @@ if (_owner == NULL && Atomic::replace_if_null(Self, &_owner)) { _EntryList = Node._next ; RawMonitor_lock->unlock() ; - return OS_OK ; + return ; } RawMonitor_lock->unlock() ; - while (Node.TState == ObjectWaiter::TS_ENTER) { + while (Node.TState == QNode::TS_ENTER) { Self->_ParkEvent->park() ; } } } -int JvmtiRawMonitor::SimpleExit (Thread * Self) { +void JvmtiRawMonitor::SimpleExit (Thread * Self) { guarantee (_owner == Self, "invariant") ; - OrderAccess::release_store(&_owner, (void*)NULL) ; + OrderAccess::release_store(&_owner, (Thread*)NULL) ; OrderAccess::fence() ; - if (_EntryList == NULL) return OS_OK ; - ObjectWaiter * w ; + if (_EntryList == NULL) return ; + QNode * w ; RawMonitor_lock->lock_without_safepoint_check() ; w = _EntryList ; @@ -166,27 +159,27 @@ } RawMonitor_lock->unlock() ; if (w != NULL) { - guarantee (w ->TState == ObjectWaiter::TS_ENTER, "invariant") ; + guarantee (w ->TState == QNode::TS_ENTER, "invariant") ; // Once we set TState to TS_RUN the waiting thread can complete // SimpleEnter and 'w' is pointing into random stack space. So we have // to ensure we extract the ParkEvent (which is in type-stable memory) // before we set the state, and then don't access 'w'. ParkEvent * ev = w->_event ; OrderAccess::loadstore(); - w->TState = ObjectWaiter::TS_RUN ; + w->TState = QNode::TS_RUN ; OrderAccess::fence() ; ev->unpark() ; } - return OS_OK ; + return ; } int JvmtiRawMonitor::SimpleWait (Thread * Self, jlong millis) { guarantee (_owner == Self , "invariant") ; guarantee (_recursions == 0, "invariant") ; - ObjectWaiter Node (Self) ; + QNode Node (Self) ; Node._notified = 0 ; - Node.TState = ObjectWaiter::TS_WAIT ; + Node.TState = QNode::TS_WAIT ; RawMonitor_lock->lock_without_safepoint_check() ; Node._next = _WaitSet ; @@ -208,12 +201,12 @@ // as TState is volatile and the lock-unlock operators are // serializing (barrier-equivalent). - if (Node.TState == ObjectWaiter::TS_WAIT) { + if (Node.TState == QNode::TS_WAIT) { RawMonitor_lock->lock_without_safepoint_check() ; - if (Node.TState == ObjectWaiter::TS_WAIT) { + if (Node.TState == QNode::TS_WAIT) { // Simple O(n) unlink, but performance isn't critical here. - ObjectWaiter * p ; - ObjectWaiter * q = NULL ; + QNode * p ; + QNode * q = NULL ; for (p = _WaitSet ; p != &Node; p = p->_next) { q = p ; } @@ -225,12 +218,12 @@ guarantee (p == q->_next, "invariant") ; q->_next = p->_next ; } - Node.TState = ObjectWaiter::TS_RUN ; + Node.TState = QNode::TS_RUN ; } RawMonitor_lock->unlock() ; } - guarantee (Node.TState == ObjectWaiter::TS_RUN, "invariant") ; + guarantee (Node.TState == QNode::TS_RUN, "invariant") ; SimpleEnter (Self) ; guarantee (_owner == Self, "invariant") ; @@ -238,9 +231,9 @@ return ret ; } -int JvmtiRawMonitor::SimpleNotify (Thread * Self, bool All) { +void JvmtiRawMonitor::SimpleNotify (Thread * Self, bool All) { guarantee (_owner == Self, "invariant") ; - if (_WaitSet == NULL) return OS_OK ; + if (_WaitSet == NULL) return ; // We have two options: // A. Transfer the threads from the WaitSet to the EntryList @@ -252,29 +245,29 @@ ParkEvent * ev = NULL ; // consider using a small auto array ... RawMonitor_lock->lock_without_safepoint_check() ; for (;;) { - ObjectWaiter * w = _WaitSet ; + QNode * w = _WaitSet ; if (w == NULL) break ; _WaitSet = w->_next ; if (ev != NULL) { ev->unpark(); ev = NULL; } ev = w->_event ; OrderAccess::loadstore() ; - w->TState = ObjectWaiter::TS_RUN ; + w->TState = QNode::TS_RUN ; OrderAccess::storeload(); if (!All) break ; } RawMonitor_lock->unlock() ; if (ev != NULL) ev->unpark(); - return OS_OK ; + return ; } // Any JavaThread will enter here with state _thread_blocked -int JvmtiRawMonitor::raw_enter(TRAPS) { +void JvmtiRawMonitor::raw_enter(Thread * Self) { void * Contended ; - + JavaThread * jt = NULL; // don't enter raw monitor if thread is being externally suspended, it will // surprise the suspender if a "suspended" thread can still enter monitor - JavaThread * jt = (JavaThread *)THREAD; - if (THREAD->is_Java_thread()) { + if (Self->is_Java_thread()) { + jt = (JavaThread*) Self; jt->SR_lock()->lock_without_safepoint_check(); while (jt->is_external_suspend()) { jt->SR_lock()->unlock(); @@ -282,150 +275,136 @@ jt->SR_lock()->lock_without_safepoint_check(); } // guarded by SR_lock to avoid racing with new external suspend requests. - Contended = Atomic::cmpxchg(THREAD, &_owner, (void*)NULL); + Contended = Atomic::cmpxchg(jt, &_owner, (Thread*)NULL); jt->SR_lock()->unlock(); } else { - Contended = Atomic::cmpxchg(THREAD, &_owner, (void*)NULL); + Contended = Atomic::cmpxchg(Self, &_owner, (Thread*)NULL); } - if (Contended == THREAD) { + if (Contended == Self) { _recursions ++ ; - return OM_OK ; + return ; } if (Contended == NULL) { - guarantee (_owner == THREAD, "invariant") ; + guarantee (_owner == Self, "invariant") ; guarantee (_recursions == 0, "invariant") ; - return OM_OK ; + return ; } - THREAD->set_current_pending_monitor(this); + Self->set_current_pending_raw_monitor(this); - if (!THREAD->is_Java_thread()) { - // No other non-Java threads besides VM thread would acquire - // a raw monitor. - assert(THREAD->is_VM_thread(), "must be VM thread"); - SimpleEnter (THREAD) ; - } else { - guarantee (jt->thread_state() == _thread_blocked, "invariant") ; - for (;;) { - jt->set_suspend_equivalent(); - // cleared by handle_special_suspend_equivalent_condition() or - // java_suspend_self() - SimpleEnter (THREAD) ; - - // were we externally suspended while we were waiting? - if (!jt->handle_special_suspend_equivalent_condition()) break ; + if (!Self->is_Java_thread()) { + SimpleEnter (Self) ; + } else { + guarantee (jt->thread_state() == _thread_blocked, "invariant") ; + for (;;) { + jt->set_suspend_equivalent(); + // cleared by handle_special_suspend_equivalent_condition() or + // java_suspend_self() + SimpleEnter (jt) ; - // This thread was externally suspended - // - // This logic isn't needed for JVMTI raw monitors, - // but doesn't hurt just in case the suspend rules change. This - // logic is needed for the JvmtiRawMonitor.wait() reentry phase. - // We have reentered the contended monitor, but while we were - // waiting another thread suspended us. We don't want to reenter - // the monitor while suspended because that would surprise the - // thread that suspended us. - // - // Drop the lock - - SimpleExit (THREAD) ; + // were we externally suspended while we were waiting? + if (!jt->handle_special_suspend_equivalent_condition()) break ; - jt->java_suspend_self(); - } + // This thread was externally suspended + // We have reentered the contended monitor, but while we were + // waiting another thread suspended us. We don't want to reenter + // the monitor while suspended because that would surprise the + // thread that suspended us. + // + // Drop the lock + SimpleExit (jt) ; - assert(_owner == THREAD, "Fatal error with monitor owner!"); - assert(_recursions == 0, "Fatal error with monitor recursions!"); + jt->java_suspend_self(); + } } - THREAD->set_current_pending_monitor(NULL); + Self->set_current_pending_raw_monitor(NULL); + + guarantee (_owner == Self, "invariant") ; guarantee (_recursions == 0, "invariant") ; - return OM_OK; } -// Used mainly for JVMTI raw monitor implementation -// Also used for JvmtiRawMonitor::wait(). -int JvmtiRawMonitor::raw_exit(TRAPS) { - if (THREAD != _owner) { - return OM_ILLEGAL_MONITOR_STATE; +int JvmtiRawMonitor::raw_exit(Thread * Self) { + if (Self != _owner) { + return M_ILLEGAL_MONITOR_STATE; } if (_recursions > 0) { --_recursions ; - return OM_OK ; + } else { + SimpleExit (Self) ; } - void * List = _EntryList ; - SimpleExit (THREAD) ; - - return OM_OK; + return M_OK; } -// Used for JVMTI raw monitor implementation. // All JavaThreads will enter here with state _thread_blocked -int JvmtiRawMonitor::raw_wait(jlong millis, bool interruptible, TRAPS) { - if (THREAD != _owner) { - return OM_ILLEGAL_MONITOR_STATE; +int JvmtiRawMonitor::raw_wait(jlong millis, bool interruptible, Thread * Self) { + if (Self != _owner) { + return M_ILLEGAL_MONITOR_STATE; } // To avoid spurious wakeups we reset the parkevent -- This is strictly optional. // The caller must be able to tolerate spurious returns from raw_wait(). - THREAD->_ParkEvent->reset() ; + Self->_ParkEvent->reset() ; OrderAccess::fence() ; + JavaThread * jt = NULL; // check interrupt event if (interruptible) { - assert(THREAD->is_Java_thread(), "Only JavaThreads can be interruptible"); - JavaThread* jt = (JavaThread*) THREAD; + assert(Self->is_Java_thread(), "Only JavaThreads can be interruptible"); + jt = (JavaThread*) Self; if (jt->is_interrupted(true)) { - return OM_INTERRUPTED; + return M_INTERRUPTED; } + } else { + assert(!Self->is_Java_thread(), "JavaThreads must be interuptible"); } intptr_t save = _recursions ; _recursions = 0 ; _waiters ++ ; - if (THREAD->is_Java_thread()) { - guarantee (((JavaThread *) THREAD)->thread_state() == _thread_blocked, "invariant") ; - ((JavaThread *)THREAD)->set_suspend_equivalent(); + if (Self->is_Java_thread()) { + guarantee (jt->thread_state() == _thread_blocked, "invariant") ; + jt->set_suspend_equivalent(); } - int rv = SimpleWait (THREAD, millis) ; + int rv = SimpleWait (Self, millis) ; _recursions = save ; _waiters -- ; - guarantee (THREAD == _owner, "invariant") ; - if (THREAD->is_Java_thread()) { - JavaThread * jSelf = (JavaThread *) THREAD ; + guarantee (Self == _owner, "invariant") ; + if (Self->is_Java_thread()) { for (;;) { - if (!jSelf->handle_special_suspend_equivalent_condition()) break ; - SimpleExit (THREAD) ; - jSelf->java_suspend_self(); - SimpleEnter (THREAD) ; - jSelf->set_suspend_equivalent() ; + if (!jt->handle_special_suspend_equivalent_condition()) break ; + SimpleExit (jt) ; + jt->java_suspend_self(); + SimpleEnter (jt) ; + jt->set_suspend_equivalent() ; } + guarantee (jt == _owner, "invariant") ; } - guarantee (THREAD == _owner, "invariant") ; - if (interruptible) { - JavaThread* jt = (JavaThread*) THREAD; - if (jt->is_interrupted(true)) { - return OM_INTERRUPTED; - } + if (interruptible && jt->is_interrupted(true)) { + return M_INTERRUPTED; } - return OM_OK ; + + return M_OK ; } -int JvmtiRawMonitor::raw_notify(TRAPS) { - if (THREAD != _owner) { - return OM_ILLEGAL_MONITOR_STATE; +int JvmtiRawMonitor::raw_notify(Thread * Self) { + if (Self != _owner) { + return M_ILLEGAL_MONITOR_STATE; } - SimpleNotify (THREAD, false) ; - return OM_OK; + SimpleNotify (Self, false) ; + return M_OK; } -int JvmtiRawMonitor::raw_notifyAll(TRAPS) { - if (THREAD != _owner) { - return OM_ILLEGAL_MONITOR_STATE; +int JvmtiRawMonitor::raw_notifyAll(Thread * Self) { + if (Self != _owner) { + return M_ILLEGAL_MONITOR_STATE; } - SimpleNotify (THREAD, true) ; - return OM_OK; + SimpleNotify (Self, true) ; + return M_OK; } diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/prims/jvmtiRawMonitor.hpp --- a/src/hotspot/share/prims/jvmtiRawMonitor.hpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/prims/jvmtiRawMonitor.hpp Mon Oct 07 18:44:53 2019 -0400 @@ -25,7 +25,8 @@ #ifndef SHARE_PRIMS_JVMTIRAWMONITOR_HPP #define SHARE_PRIMS_JVMTIRAWMONITOR_HPP -#include "runtime/objectMonitor.hpp" +#include "memory/allocation.hpp" +#include "runtime/park.hpp" #include "utilities/growableArray.hpp" // @@ -33,31 +34,69 @@ // // Used by JVMTI methods: All RawMonitor methods (CreateRawMonitor, EnterRawMonitor, etc.) // -// Wrapper for ObjectMonitor class that saves the Monitor's name +// A simplified version of the ObjectMonitor code. // -class JvmtiRawMonitor : public ObjectMonitor { -private: +class JvmtiRawMonitor : public CHeapObj { + + // Helper class to allow Threads to be linked into queues. + // This is a stripped down version of ObjectWaiter. + class QNode : public StackObj { + friend class JvmtiRawMonitor; + enum TStates { TS_READY, TS_RUN, TS_WAIT, TS_ENTER }; + QNode* volatile _next; + QNode* volatile _prev; + ParkEvent * _event; + volatile int _notified; + volatile TStates TState; + + QNode(Thread* thread); + }; + + Thread* volatile _owner; // pointer to owning thread + volatile int _recursions; // recursion count, 0 for first entry + QNode* volatile _EntryList; // Threads blocked on entry or reentry. + // The list is actually composed of nodes, + // acting as proxies for Threads. + QNode* volatile _WaitSet; // Threads wait()ing on the monitor + volatile jint _waiters; // number of waiting threads int _magic; char * _name; // JVMTI_RM_MAGIC is set in contructor and unset in destructor. enum { JVMTI_RM_MAGIC = (int)(('T' << 24) | ('I' << 16) | ('R' << 8) | 'M') }; - int SimpleEnter (Thread * Self) ; - int SimpleExit (Thread * Self) ; + void SimpleEnter (Thread * Self) ; + void SimpleExit (Thread * Self) ; int SimpleWait (Thread * Self, jlong millis) ; - int SimpleNotify (Thread * Self, bool All) ; + void SimpleNotify(Thread * Self, bool All) ; public: + + // return codes + enum { + M_OK, // no error + M_ILLEGAL_MONITOR_STATE, // IllegalMonitorStateException + M_INTERRUPTED // Thread.interrupt() + }; + + // Non-aborting operator new + void* operator new(size_t size) throw() { + return CHeapObj::operator new(size, std::nothrow); + } + JvmtiRawMonitor(const char *name); ~JvmtiRawMonitor(); - int raw_enter(TRAPS); - int raw_exit(TRAPS); - int raw_wait(jlong millis, bool interruptable, TRAPS); - int raw_notify(TRAPS); - int raw_notifyAll(TRAPS); - int magic() { return _magic; } - const char *get_name() { return _name; } + + Thread * owner() const { return _owner; } + void set_owner(Thread * owner) { _owner = owner; } + int recursions() const { return _recursions; } + void raw_enter(Thread * Self); + int raw_exit(Thread * Self); + int raw_wait(jlong millis, bool interruptable, Thread * Self); + int raw_notify(Thread * Self); + int raw_notifyAll(Thread * Self); + int magic() const { return _magic; } + const char *get_name() const { return _name; } bool is_valid(); }; diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/runtime/objectMonitor.hpp --- a/src/hotspot/share/runtime/objectMonitor.hpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/runtime/objectMonitor.hpp Mon Oct 07 18:44:53 2019 -0400 @@ -43,7 +43,6 @@ class ObjectWaiter : public StackObj { public: enum TStates { TS_UNDEF, TS_READY, TS_RUN, TS_WAIT, TS_ENTER, TS_CXQ }; - enum Sorted { PREPEND, APPEND, SORTED }; ObjectWaiter* volatile _next; ObjectWaiter* volatile _prev; Thread* _thread; @@ -51,7 +50,6 @@ ParkEvent * _event; volatile int _notified; volatile TStates TState; - Sorted _Sorted; // List placement disposition bool _active; // Contention monitoring is enabled public: ObjectWaiter(Thread* thread); @@ -68,10 +66,6 @@ // WARNING: This is a very sensitive and fragile class. DO NOT make any // changes unless you are fully aware of the underlying semantics. // -// Class JvmtiRawMonitor currently inherits from ObjectMonitor so -// changes in this class must be careful to not break JvmtiRawMonitor. -// These two subsystems should be separated. -// // ObjectMonitor Layout Overview/Highlights/Restrictions: // // - The _header field must be at offset 0 because the displaced header @@ -127,16 +121,6 @@ // in a 64-bit JVM. class ObjectMonitor { - public: - enum { - OM_OK, // no error - OM_SYSTEM_ERROR, // operating system error - OM_ILLEGAL_MONITOR_STATE, // IllegalMonitorStateException - OM_INTERRUPTED, // Thread.interrupt() - OM_TIMED_OUT // Object.wait() timed out - }; - - private: friend class ObjectSynchronizer; friend class ObjectWaiter; friend class VMStructs; @@ -158,16 +142,13 @@ DEFINE_PAD_MINUS_SIZE(0, DEFAULT_CACHE_LINE_SIZE, sizeof(volatile markWord) + sizeof(void* volatile) + sizeof(ObjectMonitor *)); - protected: // protected for JvmtiRawMonitor void* volatile _owner; // pointer to owning thread OR BasicLock - private: volatile jlong _previous_owner_tid; // thread id of the previous owner of the monitor - protected: // protected for JvmtiRawMonitor volatile intptr_t _recursions; // recursion count, 0 for first entry ObjectWaiter* volatile _EntryList; // Threads blocked on entry or reentry. // The list is actually composed of WaitNodes, // acting as proxies for Threads. - private: + ObjectWaiter* volatile _cxq; // LL of recently-arrived threads blocked on entry. Thread* volatile _succ; // Heir presumptive thread - used for futile wakeup throttling Thread* volatile _Responsible; diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/runtime/thread.cpp --- a/src/hotspot/share/runtime/thread.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/runtime/thread.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -258,6 +258,7 @@ _current_pending_monitor = NULL; _current_pending_monitor_is_from_java = true; _current_waiting_monitor = NULL; + _current_pending_raw_monitor = NULL; _num_nested_signal = 0; om_free_list = NULL; om_free_count = 0; @@ -3847,7 +3848,7 @@ // Create the VMThread { TraceTime timer("Start VMThread", TRACETIME_LOG(Info, startuptime)); - VMThread::create(); + VMThread::create(); Thread* vmthread = VMThread::vm_thread(); if (!os::create_thread(vmthread, os::vm_thread)) { diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/runtime/thread.hpp --- a/src/hotspot/share/runtime/thread.hpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/runtime/thread.hpp Mon Oct 07 18:44:53 2019 -0400 @@ -62,6 +62,7 @@ class ThreadsList; class ThreadsSMRSupport; +class JvmtiRawMonitor; class JvmtiThreadState; class ThreadStatistics; class ConcurrentLocksDump; @@ -404,6 +405,9 @@ ObjectMonitor* _current_pending_monitor; // ObjectMonitor this thread // is waiting to lock bool _current_pending_monitor_is_from_java; // locking is from Java code + JvmtiRawMonitor* _current_pending_raw_monitor; // JvmtiRawMonitor this thread + // is waiting to lock + // ObjectMonitor on which this thread called Object.wait() ObjectMonitor* _current_waiting_monitor; @@ -640,6 +644,14 @@ _current_waiting_monitor = monitor; } + // For tracking the Jvmti raw monitor the thread is pending on. + JvmtiRawMonitor* current_pending_raw_monitor() { + return _current_pending_raw_monitor; + } + void set_current_pending_raw_monitor(JvmtiRawMonitor* monitor) { + _current_pending_raw_monitor = monitor; + } + // GC support // Apply "f->do_oop" to all root oops in "this". // Used by JavaThread::oops_do. @@ -786,7 +798,7 @@ public: volatile intptr_t _Stalled; volatile int _TypeTag; - ParkEvent * _ParkEvent; // for synchronized() + ParkEvent * _ParkEvent; // for Object monitors and JVMTI raw monitors ParkEvent * _MuxEvent; // for low-level muxAcquire-muxRelease int NativeSyncRecursion; // diagnostic diff -r 43f63f904bbc -r 165b193b30dd src/hotspot/share/services/threadService.cpp --- a/src/hotspot/share/services/threadService.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/src/hotspot/share/services/threadService.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -32,6 +32,7 @@ #include "oops/objArrayKlass.hpp" #include "oops/objArrayOop.inline.hpp" #include "oops/oop.inline.hpp" +#include "prims/jvmtiRawMonitor.hpp" #include "runtime/atomic.hpp" #include "runtime/handles.inline.hpp" #include "runtime/init.hpp" @@ -217,10 +218,10 @@ } else { ObjectMonitor *enter_obj = thread->current_pending_monitor(); if (enter_obj != NULL) { - // thread is trying to enter() or raw_enter() an ObjectMonitor. + // thread is trying to enter() an ObjectMonitor. obj = (oop) enter_obj->object(); + assert(obj != NULL, "ObjectMonitor should have an associated object!"); } - // If obj == NULL, then ObjectMonitor is raw which doesn't count. } Handle h(Thread::current(), obj); @@ -354,13 +355,15 @@ } } -// Find deadlocks involving object monitors and concurrent locks if concurrent_locks is true +// Find deadlocks involving raw monitors, object monitors and concurrent locks +// if concurrent_locks is true. DeadlockCycle* ThreadService::find_deadlocks_at_safepoint(ThreadsList * t_list, bool concurrent_locks) { assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint"); // This code was modified from the original Threads::find_deadlocks code. int globalDfn = 0, thisDfn; ObjectMonitor* waitingToLockMonitor = NULL; + JvmtiRawMonitor* waitingToLockRawMonitor = NULL; oop waitingToLockBlocker = NULL; bool blocked_on_monitor = false; JavaThread *currentThread, *previousThread; @@ -391,13 +394,30 @@ // When there is a deadlock, all the monitors involved in the dependency // cycle must be contended and heavyweight. So we only care about the // heavyweight monitor a thread is waiting to lock. - waitingToLockMonitor = (ObjectMonitor*)jt->current_pending_monitor(); + waitingToLockMonitor = jt->current_pending_monitor(); + // JVM TI raw monitors can also be involved in deadlocks, and we can be + // waiting to lock both a raw monitor and ObjectMonitor at the same time. + // It isn't clear how to make deadlock detection work correctly if that + // happens. + waitingToLockRawMonitor = jt->current_pending_raw_monitor(); + if (concurrent_locks) { waitingToLockBlocker = jt->current_park_blocker(); } - while (waitingToLockMonitor != NULL || waitingToLockBlocker != NULL) { + + while (waitingToLockMonitor != NULL || + waitingToLockRawMonitor != NULL || + waitingToLockBlocker != NULL) { cycle->add_thread(currentThread); - if (waitingToLockMonitor != NULL) { + // Give preference to the raw monitor + if (waitingToLockRawMonitor != NULL) { + Thread* owner = waitingToLockRawMonitor->owner(); + if (owner != NULL && // the raw monitor could be released at any time + owner->is_Java_thread()) { + // only JavaThreads can be reported here + currentThread = (JavaThread*) owner; + } + } else if (waitingToLockMonitor != NULL) { address currentOwner = (address)waitingToLockMonitor->owner(); if (currentOwner != NULL) { currentThread = Threads::owning_thread_from_monitor_owner(t_list, @@ -948,28 +968,44 @@ JavaThread* currentThread; ObjectMonitor* waitingToLockMonitor; + JvmtiRawMonitor* waitingToLockRawMonitor; oop waitingToLockBlocker; int len = _threads->length(); for (int i = 0; i < len; i++) { currentThread = _threads->at(i); - waitingToLockMonitor = (ObjectMonitor*)currentThread->current_pending_monitor(); + waitingToLockMonitor = currentThread->current_pending_monitor(); + waitingToLockRawMonitor = currentThread->current_pending_raw_monitor(); waitingToLockBlocker = currentThread->current_park_blocker(); st->cr(); st->print_cr("\"%s\":", currentThread->get_thread_name()); const char* owner_desc = ",\n which is held by"; + + // Note: As the JVM TI "monitor contended enter" event callback is executed after ObjectMonitor + // sets the current pending monitor, it is possible to then see a pending raw monitor as well. + if (waitingToLockRawMonitor != NULL) { + st->print(" waiting to lock JVM TI raw monitor " INTPTR_FORMAT, p2i(waitingToLockRawMonitor)); + Thread* owner = waitingToLockRawMonitor->owner(); + // Could be NULL as the raw monitor could be released at any time if held by non-JavaThread + if (owner != NULL) { + if (owner->is_Java_thread()) { + currentThread = (JavaThread*) owner; + st->print_cr("%s \"%s\"", owner_desc, currentThread->get_thread_name()); + } else { + st->print_cr(",\n which has now been released"); + } + } else { + st->print_cr("%s non-Java thread=" PTR_FORMAT, owner_desc, p2i(owner)); + } + } + if (waitingToLockMonitor != NULL) { st->print(" waiting to lock monitor " INTPTR_FORMAT, p2i(waitingToLockMonitor)); oop obj = (oop)waitingToLockMonitor->object(); - if (obj != NULL) { - st->print(" (object " INTPTR_FORMAT ", a %s)", p2i(obj), - obj->klass()->external_name()); + st->print(" (object " INTPTR_FORMAT ", a %s)", p2i(obj), + obj->klass()->external_name()); - if (!currentThread->current_pending_monitor_is_from_java()) { - owner_desc = "\n in JNI, which is held by"; - } - } else { - // No Java object associated - a JVMTI raw monitor - owner_desc = " (JVMTI raw monitor),\n which is held by"; + if (!currentThread->current_pending_monitor_is_from_java()) { + owner_desc = "\n in JNI, which is held by"; } currentThread = Threads::owning_thread_from_monitor_owner(t_list, (address)waitingToLockMonitor->owner()); @@ -978,7 +1014,7 @@ // that owns waitingToLockMonitor should be findable, but // if it is not findable, then the previous currentThread is // blocked permanently. - st->print("%s UNKNOWN_owner_addr=" PTR_FORMAT, owner_desc, + st->print_cr("%s UNKNOWN_owner_addr=" PTR_FORMAT, owner_desc, p2i(waitingToLockMonitor->owner())); continue; } @@ -992,11 +1028,10 @@ currentThread = java_lang_Thread::thread(ownerObj); assert(currentThread != NULL, "AbstractOwnableSynchronizer owning thread is unexpectedly NULL"); } - st->print("%s \"%s\"", owner_desc, currentThread->get_thread_name()); + st->print_cr("%s \"%s\"", owner_desc, currentThread->get_thread_name()); } st->cr(); - st->cr(); // Print stack traces bool oldJavaMonitorsInStackTrace = JavaMonitorsInStackTrace; diff -r 43f63f904bbc -r 165b193b30dd test/hotspot/jtreg/vmTestbase/nsk/jvmti/RawMonitorWait/rawmnwait005/rawmnwait005.cpp --- a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/RawMonitorWait/rawmnwait005/rawmnwait005.cpp Mon Oct 07 13:56:11 2019 -0700 +++ b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/RawMonitorWait/rawmnwait005/rawmnwait005.cpp Mon Oct 07 18:44:53 2019 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2019, 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 @@ -37,7 +37,7 @@ static jvmtiEnv *jvmti = NULL; static jvmtiCapabilities caps; static jint result = PASSED; -static jboolean printdump = JNI_FALSE; +static jboolean printdump = JNI_TRUE; static jrawMonitorID monitor; static jrawMonitorID wait_lock; static jlong wait_time; @@ -100,6 +100,8 @@ jvmtiError err; const char* const thread_name = "test thread"; + // Once we hold this monitor we know we can't get interrupted + // until we have called wait(). err = jvmti->RawMonitorEnter(monitor); if (err != JVMTI_ERROR_NONE) { printf("(RawMonitorEnter#test) unexpected error: %s (%d)\n", @@ -110,6 +112,7 @@ printf(">>> [%s] acquired lock for 'monitor' ...\n", thread_name); } + // We can't get this monitor until the main thread has called wait() on it. err = jvmti->RawMonitorEnter(wait_lock); if (err != JVMTI_ERROR_NONE) { printf("(RawMonitorEnter#wait) unexpected error: %s (%d)\n", @@ -156,6 +159,36 @@ result = STATUS_FAILED; } + // We can't reacquire this monitor until the main thread is waiting for us to + // complete. + err = jvmti->RawMonitorEnter(wait_lock); + if (err != JVMTI_ERROR_NONE) { + printf("(RawMonitorEnter#wait) unexpected error: %s (%d)\n", + TranslateError(err), err); + result = STATUS_FAILED; + return; + } + + if (printdump == JNI_TRUE) { + printf(">>> [%s] acquired lock for 'wait_lock' ...\n", thread_name); + printf(">>> [%s] notifying main thread we are done ...\n", thread_name); + } + + err = jvmti->RawMonitorNotify(wait_lock); + if (err != JVMTI_ERROR_NONE) { + printf("(RawMonitorWait#wait) unexpected error: %s (%d)\n", + TranslateError(err), err); + result = STATUS_FAILED; + return; + } + err = jvmti->RawMonitorExit(wait_lock); + if (err != JVMTI_ERROR_NONE) { + printf("(RawMonitorExit#wait) unexpected error: %s (%d)\n", + TranslateError(err), err); + result = STATUS_FAILED; + return; + } + if (printdump == JNI_TRUE) { printf(">>> [%s] all done\n", thread_name); } @@ -223,6 +256,11 @@ if (printdump == JNI_TRUE) { printf(">>> [%s] starting test thread ...\n", thread_name); } + + // This starts a daemon thread, so we need to synchronize with it + // before we terminate - else the test will end before it checks + // it was interrupted! + err = jvmti->RunAgentThread(thr, test_thread, NULL, JVMTI_THREAD_NORM_PRIORITY); if (err != JVMTI_ERROR_NONE) { @@ -244,12 +282,7 @@ printf(">>> [%s] got notification from test thread ...\n", thread_name); } - err = jvmti->RawMonitorExit(wait_lock); - if (err != JVMTI_ERROR_NONE) { - printf("(RawMonitorExit#wait) unexpected error: %s (%d)\n", - TranslateError(err), err); - return STATUS_FAILED; - } + // Keep the wait_lock so we can wait again at the end. err = jvmti->RawMonitorEnter(monitor); if (err != JVMTI_ERROR_NONE) { @@ -280,6 +313,26 @@ } if (printdump == JNI_TRUE) { + printf(">>> [%s] waiting for test thread to complete its wait and notify us ...\n", thread_name); + } + err = jvmti->RawMonitorWait(wait_lock, (jlong)0); + if (err != JVMTI_ERROR_NONE) { + printf("(RawMonitorWait#wait) unexpected error: %s (%d)\n", + TranslateError(err), err); + return STATUS_FAILED; + } + if (printdump == JNI_TRUE) { + printf(">>> [%s] got final notification from test thread ...\n", thread_name); + } + + err = jvmti->RawMonitorExit(wait_lock); + if (err != JVMTI_ERROR_NONE) { + printf("(RawMonitorExit#wait) unexpected error: %s (%d)\n", + TranslateError(err), err); + return STATUS_FAILED; + } + + if (printdump == JNI_TRUE) { printf(">>> [%s] all done\n", thread_name); }