RFR: 8366659: ObjectMonitor::wait() can deadlock with a suspension request [v13]

[Serguei Spitsyn]

On Thu, 20 Nov 2025 05:38:21 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Changes requested by lmesnik (Reviewer).
>
> @lmesnik are the JVMTI thread state transitions performed in the event posting code? If so the different order, and thus different states, would be a concern. That said we have noticed that only the timeout case seems to operate in a way that the monitor reentry can post contended_enter and contended_entered events, which seems very odd in itself. Though I will also note that the way the suspension code drops and then re-acquires the monitor, any contended_enter* events could get posted multiple times which would also be surprising.
> 
> Maintaining the event order is problematic as, with the fix to the deadlock issue, we only allow suspension during reentry, and that would mean the `monitor_waited` event would be posted whilst the thread is still suspended. To be clear(er) in the old code we would do:
> 
> wait -> suspend if needed -> resumed -> post monitor_waited -> renter monitor
> 
> whereas the new code, if we kept the placement of the event,  would do
> 
> wait -> post monitor_waited -> reenter monitor -> suspend if needed and drop monitor -> resumed -> reenter monitor
> 
> and what the proposed code actually does is
> 
> wait -> reenter monitor -> suspend if needed and drop monitor -> resumed -> reenter monitor -> post monitor_waited 
> 
> 
> Given the lack of specificity in the JVM TI spec around these different events I think any assumptions in a TCK test could be challenged, but it would still be a change in behaviour.

@dholmes-ora David, thank you for your input!

> @dholmes-ora I have done a bit of refactoring and introduced a private method post_waited_event(), which we can call in different places depending on the scenario.

I like the refactoring.

> For instance, for the timed-out case, the sequence of events is now unchanged, i.e.:
> `wait -> waited -> contended enter -> contended entered`
> 
> For the notified case, using your extended notation, it is now behaving like this:
> `wait  -> reenter monitor -> suspend if needed and drop monitor -> resumed -> post monitor_waited -> reenter monitor`

My understanding is that we need both timed-out and notified cases to do the same as David suggested above:
  `wait -> post monitor_waited -> reenter monitor -> suspend if needed and drop monitor -> resumed -> reenter monitor`
It would give us two advantages:
 - timed-out and notified cases are unified
 - avoid incompatibility risk, e.g.:
   - no new combinations of JVMTI thread state bits are introduced
   - the object monitor events order is kept same as was before (is it true?)

Is there a problem to make it unified?

> I think this is what we eventually want. @sspitsyn could you confirm/refute that it does not change thread state bits?

As I see, this still introduces new combinations of thread state bits for the notified case.

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

PR Comment: https://git.openjdk.org/jdk/pull/27040#issuecomment-3560695036