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.
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].
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."
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.
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
