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

[Serguei Spitsyn]

On Tue, 5 Mar 2024 06:30:20 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review: addressed more comments on the fix and new test
>
> 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.

> test/hotspot/jtreg/serviceability/jvmti/ObjectMonitorUsage/ObjectMonitorUsage.java line 36:
> 
>> 34:  *       - unowned object without any waiting threads
>> 35:  *       - unowned object with threads waiting to be notified
>> 36:  *       - owned object without any waiting threads
> 
> You could say "threads waiting" in all cases - it looks odd to reverse it for two cases.

Thanks, updated.

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

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