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