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

[Serguei Spitsyn]

On Wed, 21 Feb 2024 22:34:19 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review: addressed minor issue with use of []; corrected the test desctiption
>
> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/ObjectMonitorUsage.java line 42:
> 
>> 40:  *       - all the above is checked for both platform and virtual threads
>> 41:  * @requires vm.jvmti
>> 42:  * @compile ObjectMonitorUsage.java
> 
> No need to have explicit compile for the test code.

Fixed, thanks.

> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/ObjectMonitorUsage.java line 204:
> 
>> 202: 
>> 203:         // test virtual threads
>> 204:         test1(false);
> 
> shouldn't be true here?

Nice catch, thanks. Fixed now.

> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/libObjectMonitorUsage.cpp line 105:
> 
>> 103:   err = jvmti->GetObjectMonitorUsage(obj, &inf);
>> 104:   if (err == JVMTI_ERROR_MUST_POSSESS_CAPABILITY && !caps.can_get_monitor_info) {
>> 105:     return; /* Ok, it's expected */
> 
> I think we don't need this path.

Yes. Fixed, thanks.

> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/libObjectMonitorUsage.cpp line 107:
> 
>> 105:     return; /* Ok, it's expected */
>> 106:   } else if (err != JVMTI_ERROR_NONE) {
>> 107:     LOG("(GetMonitorInfo#%d) unexpected error: %s (%d)\n",
> 
> you could use check_jvmti_status

Fixed, thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498694748
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498694426
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498693585
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498693206