RFR: 8345543: Test serviceability/jvmti/vthread/StopThreadTest/StopThreadTest.java failed: expected JVMTI_ERROR_OPAQUE_FRAME instead of: 0 [v4]

[Alan Bateman]

On Thu, 16 Jan 2025 05:58:15 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> The problem is that the state of the target `vthread` is changed to `BLOCKED` too early. It is done in a constructor of helper object `JavaThreadBlockedOnMonitorEnterState` which sets the state of `vthread` to `JavaThreadStatus::BLOCKED_ON_MONITOR_ENTER`. The `vthread` is getting suspended in `try_preempt()` while calling `JvmtiVTMSTransitionDisabler::start_VTMS_transition()`, so does not become unmounted as it is expected by the failing test. Also, it seems that the base class of the `JavaThreadBlockedOnMonitorEnterState` is supporting just HotSpot  `JavaThread`'s and so, it is changing the carrier thread state only.
>> 
>> The fix is to move the definition of the `JavaThreadBlockedOnMonitorEnterState` object after block where `try_preempt()` is called.
>> 
>> Testing:
>>  - Checked the failed test does not fail with the fix: `serviceability/jvmti/vthread/StopThreadTest/StopThreadTest.java`
>>  - Ran mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: restored the set_current_pending_monitor(this) position

I suppose David does have a point that long standing behavior with the MonitorContendedEnter posted on a platform thread is done in the blocked state. In past discussions I think we questioned if this was a bug or not. The spec is that the event is posted when "attempting" to enter, which I think means it hasn't yet blocked. I can't immediately think of any case where an agent would care but if we are changing it then maybe we can clarify the JVMTI spec in the same release.

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

PR Comment: https://git.openjdk.org/jdk/pull/23125#issuecomment-2595918581