RFR: 8316417: ObjectMonitorIterator does not return the most recent monitor and is incorrect if no monitors exists [v5]
Axel Boldt-Christmas
aboldtch at openjdk.org
Thu Sep 21 06:14:43 UTC 2023
On Wed, 20 Sep 2023 22:37:59 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:
>> Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Removed unnecessary comment
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectSynchronizer.java line 86:
>
>> 84:
>> 85: ObjectMonitorIterator() {
>> 86: mon = new ObjectMonitor(inUseList);
>
> How did this ever work? `inUseList` is an address that points to a `MonitorList`, not an `ObjectMonitor`. Your new code has it right, but it seems that this old code would have failed either during construction, or during the first `mon.nextOM()` call.
`inUseList` will end up with the same value as `inUseListHead`. The reason the old code worked is because `getAddressField` does not type check and `reinterpret_cast<addres>(&ObjectSynchronizer::_in_use_list) == reinterpret_cast<addres>(&ObjectSynchronizer::_in_use_list._head)`
Effectively I changed this to load it correctly (regardless of what `offset_of(MonitorList, _head)` ends up being) and name the variables more appropriately.
C++ interpretation of what the java change does:
```C++
// Old code
// Type type = db.lookupType("ObjectSynchronizer");
// inUseList = type.getAddressField("_in_use_list").getValue();
address inUseList = *(reinterpret_cast<address*>(&ObjectSynchronizer::_in_use_list));
// New code
// Type objectSynchronizerType = db.lookupType("ObjectSynchronizer");
// Type monitorListType = db.lookupType("MonitorList");
// Address monitorListAddr = objectSynchronizerType.getField("_in_use_list").getStaticFieldAddress();
// inUseListHead = monitorListType.getAddressField("_head").getAddress(monitorListAddr);
address monitorListAddr = reinterpret_cast<address>(&ObjectSynchronizer::_in_use_list);
address inUseList = *(reinterpret_cast<address*>(monitorListAddr + offset_of(MonitorList, _head)));
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1332516949
More information about the serviceability-dev
mailing list