On Thu, 21 Sep 2023 06:21:25 GMT, Axel Boldt-Christmas <abold...@openjdk.org> 
wrote:

>> ObjectMonitorIterator fails to return the most resent monitor added. It 
>> start with returning the `nextOM()` ObjectMonitor from the `_head` 
>> ObjectMonitor but fails to ever return the `_head` ObjectMonitor.
>> The current implementation can also not handle that the `_head` is nullptr 
>> (no monitors in the system) and returns a null ObjectMonitorIterator. Which 
>> is interpreted as `monitor list not supported, too old hotspot VM`.
>> 
>> Changed the iterator to keep return the current monitor (starts with 
>> `_head`) and decoupled `_head == nullptr` from the question if 
>> ObjectMonitorIterator is supported. 
>> 
>> Testing:
>>   * Passes all `serviceability/sa` tests
>>   * Passed tier 1-5
>>   * Passed GHA
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

Seems to fix the two issues pointed out in JBS but I have two comments below

Thanks.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectSynchronizer.java
 line 49:

> 47:     Type monitorListType = db.lookupType("MonitorList");
> 48:     Address monitorListAddr = 
> objectSynchronizerType.getField("_in_use_list").getStaticFieldAddress();
> 49:     inUseListHead = 
> monitorListType.getAddressField("_head").getAddress(monitorListAddr);

So is `inUseListHead` effectively a "pointer" to the `_head` field of the 
`_in_use_list` `MonitorList`, which is dereferenced when we read it? Else I'm 
not seeing how we iterate the current set of in-use monitors.

test/hotspot/jtreg/serviceability/sa/TestObjectMonitorIterate.java line 1:

> 1: /*

This test still barely actually tests the iterator code.

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

PR Review: https://git.openjdk.org/jdk/pull/15782#pullrequestreview-1641218470
PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1335312311
PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1335344790

Reply via email to