RFR: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage [v5]

[Serguei Spitsyn]

On Wed, 14 Feb 2024 02:18:58 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - Merge
>>  - review: fixed issues in get_object_monitor_usage; extended test coverage in objmonusage003
>>  - Merge
>>  - review: thread in notify waiter list can't be BLOCKED
>>  - 8324677: Specification clarification needed for JVM TI GetObjectMonitorUsage
>
> src/hotspot/share/runtime/threads.cpp line 1200:
> 
>> 1198:     if (pending == monitor ||
>> 1199:         (waiting == monitor && JavaThreadStatus::BLOCKED_ON_MONITOR_ENTER ==
>> 1200:          java_lang_Thread::get_thread_status(p->vthread_or_thread()))
> 
> This code seems extremely racy. Is there anything that is stopping the target thread from running while we attempt this inspection of the oop state? If it was waiting and was interrupted or timed-out then we could mistakenly think it was still pending entry to this monitor when in fact it is not.
> It is also a shame that we have to reach up into the oop thread state to figure out if if it is blocked on re-entry ...

The function `get_pending_threads()` is executed only by the VM_GetObjectMonitorUsage operation.
I've added this assert there: 
    `assert(Thread::current()->is_VM_thread(), "Must be the VM thread");`
Does it address your concern?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1489379382