RFR: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage [v22]

[Serguei Spitsyn]

On Tue, 5 Mar 2024 08:53:42 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1507:
>> 
>>> 1505:     nWait = 0;
>>> 1506:     for (ObjectWaiter* waiter = mon->first_waiter();
>>> 1507:          waiter != nullptr && (nWait == 0 || waiter != mon->first_waiter());
>> 
>> Sorry I do not understand the logic of this line. `waiters` is just a linked-list of `ObjectWaiter`s so all we need to do is traverse it and count the number of elements.
>
> The loop is endless without this extra condition, so we are getting a test execution timeout.
> The `waiters` seems to be `circular doubly linked list` as we can see below:
> 
> inline void ObjectMonitor::AddWaiter(ObjectWaiter* node) {
>   assert(node != nullptr, "should not add null node");
>   assert(node->_prev == nullptr, "node already in list");
>   assert(node->_next == nullptr, "node already in list");
>   // put node at end of queue (circular doubly linked list)
>   if (_WaitSet == nullptr) {
>     _WaitSet = node;
>     node->_prev = node;
>     node->_next = node;
>   } else {
>     ObjectWaiter* head = _WaitSet;
>     ObjectWaiter* tail = head->_prev;
>     assert(tail->_next == head, "invariant check");
>     tail->_next = node;
>     head->_prev = node;
>     node->_next = head;
>     node->_prev = tail;
>   }
> }
> 
> I'll make sure nothing is missed and check the old code again.

Okay. Now I see why the old code did not have endless loops.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1512409266