[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage [v18]
From:       Serguei Spitsyn <sspitsyn () openjdk ! org>
Date:       2024-02-29 4:11:24
Message-ID: R_KuuMIt1-k0TLlr8fvRww--8zLxtmRkJhGSbQZ0US0=.90314c56-f8e1-4c15-ae93-d30aae36ee44 () github ! com
[Download RAW message or body]

> The implementation of the JVM TI `GetObjectMonitorUsage` does not match the spec.
> The function returns the following structure:
> 
> 
> typedef struct {
> jthread owner;
> jint entry_count;
> jint waiter_count;
> jthread* waiters;
> jint notify_waiter_count;
> jthread* notify_waiters;
> } jvmtiMonitorUsage;
> 
> 
> The following four fields are defined this way:
> 
> waiter_count  [jint] The number of threads waiting to own this monitor
> waiters  [jthread*] The waiter_count waiting threads
> notify_waiter_count  [jint]  The number of threads waiting to be notified by this \
> monitor notify_waiters  [jthread*] The notify_waiter_count threads waiting to be \
> notified 
> The `waiters` has to include all threads waiting to enter the monitor or to \
> re-enter it in `Object.wait()`. The implementation also includes the threads \
> waiting to be notified in `Object.wait()` which is wrong. The `notify_waiters` has \
> to include all threads waiting to be notified in `Object.wait()`. The \
> implementation also includes the threads waiting to re-enter the monitor in \
> `Object.wait()` which is wrong. This update makes it right.
> 
> The implementation of the JDWP command `ObjectReference.MonitorInfo (5)` is based \
> on the JVM TI `GetObjectMonitorInfo()`. This update has a tweak to keep the \
> existing behavior of this command. 
> The follwoing JVMTI vmTestbase tests are fixed to adopt to the \
> `GetObjectMonitorUsage()` correct behavior: 
> jvmti/GetObjectMonitorUsage/objmonusage001
> jvmti/GetObjectMonitorUsage/objmonusage003
> 
> 
> The following JVMTI JCK tests have to be fixed to adopt to correct behavior:
> 
> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101.html
> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101a.html
> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102.html
> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102a.html
> 
> 
> 
> A JCK bug will be filed and the tests have to be added into the JCK problem list \
> located in the closed repository. 
> Also, please see and review the related CSR:
> [8324677](https://bugs.openjdk.org/browse/JDK-8324677): incorrect implementation of \
> JVM TI GetObjectMonitorUsage 
> The Release-Note is:
> [8325314](https://bugs.openjdk.org/browse/JDK-8325314): Release Note: incorrect \
> implementation of JVM TI GetObjectMonitorUsage 
> Testing:
> - tested with mach5 tiers 1-6

Serguei Spitsyn has updated the pull request with a new target base due to a merge or \
a rebase. The pull request now contains 19 commits:

 - resolve merge conflict for deleted file objmonusage003.cpp
 - fix a typo in libObjectMonitorUsage.cpp
 - fix potential sync gap in the test ObjectMonitorUsage
 - improve ObjectMonitorUsage test native agent output
 - improved the ObjectMonitorUsage test to make it more elegant
 - review: remove test objmonusage003; improve test ObjectMonitorUsage
 - review: addressed minor issue with use of []; corrected the test desctiption
 - review: addressed comments from David
 - fixed minimal build issue
 - cloned an nsk/jvmti test to provide test coverage for virtual threads; fixed \
                vthread related issue
 - ... and 9 more: https://git.openjdk.org/jdk/compare/5fa2bdc6...06711644

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

Changes: https://git.openjdk.org/jdk/pull/17680/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17680&range=17
  Stats: 1120 lines in 14 files changed: 684 ins; 389 del; 47 mod
  Patch: https://git.openjdk.org/jdk/pull/17680.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17680/head:pull/17680

PR: https://git.openjdk.org/jdk/pull/17680


[prev in list] [next in list] [prev in thread] [next in thread]