On Tue, 12 Mar 2024 05:13:54 GMT, David Holmes <dhol...@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