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

[Serguei Spitsyn]

On Tue, 12 Mar 2024 05:13:54 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 minor comments, updated a couple of copyright headers
>
> test/hotspot/jtreg/serviceability/jvmti/ObjectMonitorUsage/ObjectMonitorUsage.java line 352:
> 
>> 350:                     wokeupCount++;
>> 351:                     if (!notifiedAll && notifiedCount < wokeupCount) {
>> 352:                         wasSpuriousWakeup = true;
> 
> This doesn't work. First as you've realized you cannot detect a spurious wakeup when notifyAll is used. But second the `notifiedCount` is incremented on each `notify` whilst the monitor lock is still held - as it must be. So no `wait` can return until the monitor is unlocked. So unless the spurious wakeup occurs before the monitor is acquired at line 254, all notifications must be completed before even a spurious wakeup can cause `wait()` to return - at which point `notifiedCount` must equal ` NUMBER_OF_WAITING_THREADS` which can never be < `wokeupCount`.
> Given you can only detect one very specific case of a spurious wakeup, all this extra logic is just adding to the complexity and giving a false impression about what is being checked. To detect actual spurious wakeups here you need to be able to track when each thread was chosen by a `notify()` and there is no means to do that.

You are right, thanks!
Removed the incorrect spurious wakeup detection code.

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

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