On Thu, 20 Nov 2025 05:38:21 GMT, David Holmes <[email protected]> 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
