On Tue, 12 Mar 2024 05:13:54 GMT, David Holmes <[email protected]> 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
