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