RFR: 8346792: serviceability/jvmti/vthread/GetThreadState/GetThreadState.java testObjectWaitMillis failed [v7]
Serguei Spitsyn
sspitsyn at openjdk.org
Wed Jan 22 10:48:54 UTC 2025
> Below is the root cause of the bug described by Patricio:
> The root of the issue is similar to [JDK-8345543](https://bugs.openjdk.org/browse/JDK-8345543), but instead of `JavaThreadBlockedOnMonitorEnterState`, the interaction here is with its equivalent for Object.wait, `JavaThreadInObjectWaitState`.
>
> We change the `Thread.State` of the carrier to `TIMED_WAITING` before we try preempting the `vthread`. So we can see the `Thread.State` of the vthread as `TIMED_WAITING` before even calling `try_preempt()` (when the state is `RUNNING` we return the `Thread.State` of the carrier). Then when we check for `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER`, we caught the `vthread` in `afterYield()` still with the transitional state of `TIMED_WAITING`, which is mapped to `Thread.State.RUNNABLE`.
>
> The fix is to delay the state change to `JavaThreadStatus::IN_OBJECT_WAIT*` the point after block with the `try_preempt()` call. The fix is not elegant and hacky but I do not see a better solution. The ugliness comes from the fact that the `ObjectMonitor::wait()` is called from two places: `ObjectSynchronizer::wait()` and `ObjectSynchronizer::waitUninterruptibly()`. We do not need the `JavaThreadInObjectWaitState` in the second cases, so the object is defined in the `JVM_MonitorWait()` but not in the `ObjectMonitor::wait()`.
> Maybe, deep refactoring the`JavaThreadInObjectWaitState` base class `JavaThreadStatusChanger` could be a better solution but it is going to be much more intrusive.
>
> Testing:
> - Checked the failed test does not fail with the fix: `serviceability/jvmti/vthread/GetThreadState/GetThreadState.java`
> - Ran mach5 tiers 1-6
Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
- Merge
- review: preserve platform thread behavior
- review: add some useful comments
- review: restored the current->set_current_waiting_monitor(this) position as it was originally
- review: move set_current_waiting_monitor down; cleanup DBG ifdefs
- removed empty line with trailing spaces
- review: refix with a new constructor arg: iterruptible
- 8346792: serviceability/jvmti/vthread/GetThreadState/GetThreadState.java testObjectWaitMillis failed
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/23126/files
- new: https://git.openjdk.org/jdk/pull/23126/files/9be2cbe0..eeff9720
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=23126&range=06
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=23126&range=05-06
Stats: 17606 lines in 2070 files changed: 8012 ins; 4807 del; 4787 mod
Patch: https://git.openjdk.org/jdk/pull/23126.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23126/head:pull/23126
PR: https://git.openjdk.org/jdk/pull/23126
More information about the hotspot-runtime-dev
mailing list