Bug ID: JDK-8185164 GetOwnedMonitorInfo() returns incorrect owned monitor

Type: Bug
Component: hotspot
Sub-Component: jvmti
Affected Version: 10

Priority: P4
Status: Resolved
Resolution: Fixed

Submitted: 2017-07-25
Updated: 2018-02-15
Resolved: 2017-08-07

JDK 10	JDK 11	JDK 8
10 b21Fixed	11Resolved	8u162Fixed

GetOwnedMonitorInfo JVMTI function returns a monitor which is not yet owned when it is called at MonitorContendedEnter JVMTI event.

I attached reproducer in this tichet. Please read README.md.

GetOwnedMonitorInfo() should not return a pending monitor.

[ @David ] Yes, I know it was my fault as I did not notice this bug had the 'Fix Version' 11. I agree, we have to pay more attention to the 'Fix Version' when pushing fixes for 10.
08-08-2017
Dan, thank you for fixing the errant backport issue!
07-08-2017
Copied from errant backport bug JDK-8185926: hgupdate HG Updates added a comment - 35 minutes ago URL: http://hg.openjdk.java.net/jdk10/hs/hotspot/rev/f2ec523d900b User: sspitsyn Date: 2017-08-07 20:53:58 +0000
07-08-2017
Given JvmtiEnvBase::get_owned_monitors has: assert((SafepointSynchronize::is_at_safepoint() \|\| is_thread_fully_suspended(java_thread, false, &debug_bits)), "at safepoint or target thread is suspended"); it seems to me that (as I often comment about API calls made from callbacks) there is no realization that this code might be called via a callback during the monitor acquisition process, and hence that the "pending monitor" state might not be stable. The suggested fix addresses the problem and I don't see any other uses of the pending monitor that might be impacted negatively.
01-08-2017
So the JVM/TI GetOwnedMonitorInfo() wrapper calls JvmtiEnv::GetOwnedMonitorInfo() which determines whether to use JvmtiEnvBase::get_owned_monitors() directly or via a VM operation. JvmtiEnvBase::get_owned_monitors() calls JvmtiEnvBase::get_locked_objects_in_frame() which has a check for a pending monitor: if (pending_obj == obj) { // the thread is pending on this monitor so it isn't really owned continue; } However, that check is dependent on the earlier code: oop pending_obj = NULL; { // save object of current enter() call (if any) for later comparison ObjectMonitor *mon = java_thread->current_pending_monitor(); if (mon != NULL) { pending_obj = (oop)mon->object(); } } Since the JavaThread::_current_pending_monitor field hasn't been set at the time that the MONITOR_CONTENDED_ENTER event is posted, the pending monitor logic in JvmtiEnvBase::get_locked_objects_in_frame() is fooled. The problem is that the _current_pending_monitor field setting is carefully placed: { // Change java thread status to indicate blocked on monitor enter. JavaThreadBlockedOnMonitorEnterState jtbmes(jt, this); DTRACE_MONITOR_PROBE(contended__enter, this, object(), jt); if (JvmtiExport::should_post_monitor_contended_enter()) { JvmtiExport::post_monitor_contended_enter(jt, this); // The current thread does not yet own the monitor and does not // yet appear on any queues that would get it made the successor. // This means that the JVMTI_EVENT_MONITOR_CONTENDED_ENTER event // handler cannot accidentally consume an unpark() meant for the // ParkEvent associated with this ObjectMonitor. } OSThreadContendState osts(Self->osthread()); ThreadBlockInVM tbivm(jt); Self->set_current_pending_monitor(this); // TODO-FIXME: change the following for(;;) loop to straight-line code. for (;;) { jt->set_suspend_equivalent(); // cleared by handle_special_suspend_equivalent_condition() // or java_suspend_self() EnterI(THREAD); The field is set after OSThreadContendState and ThreadBlockInVM helper creation which changes a bunch of visible state. We'll have to carefully examine the places that query the _current_pending_monitor field to make sure that code won't be confused by this value being set in a different thread state.
25-07-2017
Moving from hotspot/svc -> hotspot/jvmti since GetOwnedMonitorInfo is a JVM/TI API.
25-07-2017