RFR: 8316417: ObjectMonitorIterator does not return the most recent monitor and is incorrect if no monitors exists [v3]

Axel Boldt-Christmas aboldtch at openjdk.org
Tue Sep 19 06:51:48 UTC 2023


On Mon, 18 Sep 2023 18:46:19 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:
>> 
>>   Copyright changes too
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectSynchronizer.java line 51:
> 
>> 49:     Address monitorListAddr = objectSynchronizerType.getField("_in_use_list").getStaticFieldAddress();
>> 50:     inUseListHead = monitorListType.getAddressField("_head").getAddress(monitorListAddr);
>> 51:     isSupported = true;
> 
> I understand the changes below, but I'm a bit less clear why the above changes were needed. Is this to help simplyfy the iterator logic below and make it easier to check for a null list?
> 
> I'm not so sure I like the `isSupported` logic. It seems we should throw an exception at some point if initialize() fails. I think in other classes when some part of initialize() fails, we just allow the NPE to happen when later logic tries to access the value. The check in objectMontitorIterator() was probably a somewhat misguided attempted to be compatible with older VMs, which is not something we really strive for anymore.

`inUseListHead` will be null both if `initialize` throws an exception and if the monitor list is null. The previous semantics was to return null if we failed to get the monitor list (because `initialize` failed). But with only  `inUseListHead` we cannot differentiate between the two states. If it is null and `initialize` did not fail, an empty `ObjectMontiorIterator` should be returned instead of null. 

But if it is as you say that this is not a supported use case we could just return an empty `ObjectMontiorIterator` for both cases and rely on the handling of the exception thrown by `initialize`. 

I did not want to change the interface. But I'll follow your initiative here and remove the `null` return from `objectMonitorIterator()`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1329649000


More information about the serviceability-dev mailing list