On Thu, 18 Nov 2021 09:34:13 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> The test fails when the target JavaThread has is_exiting() status. In such a
>> case the JvmtiExport::cleanup_thread(this) has already made a clean up of
>> its jvmtiThreadState, so the JavaThread address returned by
>> _state->get_thread() is 0xbabababababababa.
>> The fix is to add a check for is_exiting() status into handshake closure
>> do_thread() early.
>> There following handshake closures are fixed by this update:
>> - UpdateForPopTopFrameClosure
>> - SetForceEarlyReturn
>> - SetFramePopClosure
>
> Serguei Spitsyn has updated the pull request incrementally with one
> additional commit since the last revision:
>
> get rid of the checks in jvmti handshakes: java_thread->threadObj() == NULL
I don't see a reason for the change in `SetForceEarlyReturn::doit()`,
but I'm okay with the other changes.
src/hotspot/share/prims/jvmtiEnvBase.cpp line 1401:
> 1399: if (!self) {
> 1400: if (!java_thread->is_suspended()) {
> 1401: _result = JVMTI_ERROR_THREAD_NOT_SUSPENDED;
I don't see an obvious reason for this `is_exiting()` check.
src/hotspot/share/prims/jvmtiEnvBase.cpp line 1625:
> 1623: return; /* JVMTI_ERROR_THREAD_NOT_ALIVE (default) */
> 1624: }
> 1625: assert(_state->get_thread() == java_thread, "Must be");
The `assert()` on L1625 is subject to the same race as the original site.
This `is_exiting()` check is made under the protection of the
`JvmtiThreadState_lock` so it is sufficient to protect that `assert()`.
-------------
Changes requested by dcubed (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6440
