On Sat, 10 Feb 2024 04:06:37 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> The implementation of the JVM TI `GetObjectMonitorUsage` does not match the 
>> spec.
>> The function returns the following structure:
>> 
>> 
>> typedef struct {
>>     jthread owner;
>>     jint entry_count;
>>     jint waiter_count;
>>     jthread* waiters;
>>     jint notify_waiter_count;
>>     jthread* notify_waiters;
>> } jvmtiMonitorUsage;
>> 
>> 
>> The following four fields are defined this way:
>> 
>> waiter_count  [jint] The number of threads waiting to own this monitor
>> waiters  [jthread*] The waiter_count waiting threads
>> notify_waiter_count  [jint]  The number of threads waiting to be notified by 
>> this monitor
>> notify_waiters  [jthread*] The notify_waiter_count threads waiting to be 
>> notified
>> 
>> The `waiters` has to include all threads waiting to enter the monitor or to 
>> re-enter it in `Object.wait()`.
>> The implementation also includes the threads waiting to be notified in 
>> `Object.wait()` which is wrong.
>> The `notify_waiters` has to include all threads waiting to be notified in 
>> `Object.wait()`.
>> The implementation also includes the threads waiting to re-enter the monitor 
>> in `Object.wait()` which is wrong.
>> This update makes it right.
>> 
>> The implementation of the JDWP command `ObjectReference.MonitorInfo (5)` is 
>> based on the JVM TI `GetObjectMonitorInfo()`. This update has a tweak to 
>> keep the existing behavior of this command.
>> 
>> The follwoing JVMTI vmTestbase tests are fixed to adopt to the 
>> `GetObjectMonitorUsage()` correct behavior:
>> 
>>   jvmti/GetObjectMonitorUsage/objmonusage001
>>   jvmti/GetObjectMonitorUsage/objmonusage003
>> 
>> 
>> The following JVMTI JCK tests have to be fixed to adopt to correct behavior:
>> 
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101a.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102a.html
>> 
>> 
>> 
>> A JCK bug will be filed and the tests have to be added into the JCK problem 
>> list located in the closed repository.
>> 
>> Also, please see and review the related CSR:
>>  [8324677](https://bugs.openjdk.org/browse/JDK-8324677): incorrect 
>> implementation of JVM TI GetObjectMonitorUsage
>> 
>> The Release-Note is:
>> [8325314](https://bugs.openjdk.org/browse/JDK-8325314): Release Note: 
>> incorrect implementation of JVM TI GetObjectMonitorUsage
>>  
>> Testing:
>>  - tested with mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: fixed issues in get_object_monitor_usage; extended test coverage in 
> objmonusage003

I've pushed an update addressing review comments from Dan/David.
This is a short list describing the updates:
 - now the function `Threads::get_pending_threads()` in the `threads.cpp` is 
supporting the monitor re-enter case
 - introduced the function `oop JavaThread::vthread_or_thread()` used in the 
function `Threads::get_pending_threads()`
 - the fix in `jvmtiEnvBase.cpp` has been re-arranged to calculate early the 
right sizes of the `waiters` and `notify_waiters` array
 - the NSK test 
`test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage003` 
was extended to provide coverage suggested by David. It recreates a scenario 
with several threads blocked on the monitor enter, several threads blocked on 
re-enter and several thread waiting in the `Object.wait()` to be notified.
 - some small test fixes suggested in review

The mach5 tiers 1-6 are all passed except the four known to fail JCK tests in 
the tier 6.

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

PR Comment: https://git.openjdk.org/jdk/pull/17680#issuecomment-1936927038

Reply via email to