Question about GetObjectMonitorUsage() JVMTI function

Daniel D. Daugherty daniel.daugherty at oracle.com
Mon Jun 15 15:13:02 UTC 2020 

On 6/15/20 10:45 AM, Yasumasa Suenaga wrote:
> On 2020/06/15 22:38, Daniel D. Daugherty wrote:
>> On 6/15/20 3:26 AM, David Holmes wrote:
>>> On 15/06/2020 4:02 pm, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>> On 2020/06/15 14:15, David Holmes wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> On 15/06/2020 2:49 pm, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I wonder why JvmtiEnvBase::get_object_monitor_usage() 
>>>>>> (implementation of GetObjectMonitorUsage()) does not perform at 
>>>>>> safepoint.
>>>>>
>>>>> GetObjectMonitorUsage will use a safepoint if the target is not 
>>>>> suspended:
>>>>>
>>>>> jvmtiError
>>>>> JvmtiEnv::GetObjectMonitorUsage(jobject object, jvmtiMonitorUsage* 
>>>>> info_ptr) {
>>>>>    JavaThread* calling_thread = JavaThread::current();
>>>>>    jvmtiError err = get_object_monitor_usage(calling_thread, 
>>>>> object, info_ptr);
>>>>>    if (err == JVMTI_ERROR_THREAD_NOT_SUSPENDED) {
>>>>>      // Some of the critical threads were not suspended. go to a 
>>>>> safepoint and try again
>>>>>      VM_GetObjectMonitorUsage op(this, calling_thread, object, 
>>>>> info_ptr);
>>>>>      VMThread::execute(&op);
>>>>>      err = op.result();
>>>>>    }
>>>>>    return err;
>>>>> } /* end GetObject */
>>>>
>>>> I saw this code, so I guess there are some cases when 
>>>> JVMTI_ERROR_THREAD_NOT_SUSPENDED is not returned from 
>>>> get_object_monitor_usage().
>>>>
>>>>
>>>>>> Monitor owner would be acquired from monitor object at first [1], 
>>>>>> but it would perform concurrently.
>>>>>> If owner thread is not suspended, the owner might be changed to 
>>>>>> others in subsequent code.
>>>>>>
>>>>>> For example, the owner might release the monitor before [2].
>>>>>
>>>>> The expectation is that when we find an owner thread it is either 
>>>>> suspended or not. If it is suspended then it cannot release the 
>>>>> monitor. If it is not suspended we detect that and redo the whole 
>>>>> query at a safepoint.
>>>>
>>>> I think the owner thread might resume unfortunately after 
>>>> suspending check.
>>>
>>> Yes you are right. I was thinking resuming also required a safepoint 
>>> but it only requires the Threads_lock. So yes the code is wrong.
>>
>> Which code is wrong?
>>
>> Yes, a rogue resume can happen when the GetObjectMonitorUsage() caller
>> has started the process of gathering the information while not at a
>> safepoint. Thus the information returned by GetObjectMonitorUsage()
>> might be stale, but that's a bug in the agent code.
>
> I don't think so.
>
> For example, JVMTI agent might attempt to get monitor owner from 
> sleeping thread or in native (e.g. during socket operation) thread, 
> and it might resume during GetObjectMonitorUsage() call.

A "sleeping thread" or a thread "in native" does not count as being
suspended so GetObjectMonitorUsage() will take the safepoint code
path.


> Agent code can (should) not control application threads as an observer.

The agent writer has the choice of using JVM/TI SuspendThread() and
JVM/TI ResumeThread() along with GetObjectMonitorUsage() or it can
use GetObjectMonitorUsage() without direct calls to SuspendThread()
and ResumeThread. It's the agent writer's choice.

If the agent is only interested in a single thread, then doing:

     SuspendThread()            // might safepoint
     GetObjectMonitorUsage()    // won't safepoint
     ResumeThread()             // won't safepoint

is more performant than doing just GetObjectMonitorUsage() which
is guaranteed to safepoint (implementation detail).


> IMHO GetObjectMonitorUsage() should perform at safepoint or should 
> start direct handshake immediately after getting owner thread from 
> monitor.

Handshakes are a separate matter all together. I'm talking about
the way the code works now and I don't think we've switched JVM/TI
GetObjectMonitorUsage() to use handshakes yet or have we done so?

Dan



>
>
> Yasumasa
>
>
>> Dan
>>
>>
>>>
>>>
>>>> JavaThread::is_ext_suspend_completed() is used to check thread 
>>>> state, it returns `true` when the thread is sleeping [3], or when 
>>>> it performs in native [4].
>>>
>>> Sure but if the thread is actually suspended it can't continue 
>>> execution in the VM or in Java code.
>>>
>>>>
>>>>> This appears to be an optimisation for the assumed common case 
>>>>> where threads are first suspended and then the monitors are queried.
>>>>
>>>> I agree with this, but I could find out it from JVMTI spec - it 
>>>> just says "Get information about the object's monitor."
>>>
>>> Yes it was just an implementation optimisation, nothing to do with 
>>> the spec.
>>>
>>>> GetObjectMonitorUsage() might return incorrect information in some 
>>>> case.
>>>>
>>>> It starts with finding owner thread, but the owner might be just 
>>>> before wakeup.
>>>> So I think it is more safe if GetObjectMonitorUsage() is called at 
>>>> safepoint in any case.
>>>
>>> Except we're moving away from safepoints to using Handshakes, so 
>>> this particular operation will require that the apparent owner is 
>>> Handshake-safe (by entering a handshake with it) before querying the 
>>> monitor. This would still be preferable I think to always using a 
>>> safepoint for the entire operation.
>>>
>>> Cheers,
>>> David
>>> -----
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> [3] 
>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/runtime/thread.cpp#l671
>>>> [4] 
>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/runtime/thread.cpp#l684
>>>>
>>>>
>>>>> However there is still a potential bug as the thread reported as 
>>>>> the owner may not be suspended at the time we first see it, and 
>>>>> may release the monitor, but then it may get suspended before we 
>>>>> call:
>>>>>
>>>>>   owning_thread = 
>>>>> Threads::owning_thread_from_monitor_owner(tlh.list(), owner);
>>>>>
>>>>> and so we think it is still the monitor owner and proceed to query 
>>>>> the monitor information in a racy way. This can't happen when 
>>>>> suspension itself requires a safepoint as the current thread won't 
>>>>> go to that safepoint during this code. However, if suspension is 
>>>>> implemented via a direct handshake with the target thread then we 
>>>>> have a problem.
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> [1] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/prims/jvmtiEnvBase.cpp#l973
>>>>>> [2] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/prims/jvmtiEnvBase.cpp#l996
>>

More information about the serviceability-dev mailing list