[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Integrated: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHa
From: Serguei Spitsyn <sspitsyn () openjdk ! java ! net>
Date: 2022-05-26 0:32:35
Message-ID: ppltJeFulEPHD2DUBzSVKqE261L3NwhCz3_d-F08VcE=.7acfcfce-ef60-40c5-8af3-3f895c841a42 () github ! com
[Download RAW message or body]
On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn <sspitsyn@openjdk.org> wrote:
> A part of this issue was contributed with the following changeset:
>
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcubed@openjdk.org](mailto:dcubed@openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +0000
>
> 8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes
> Reviewed-by: coleenp, sspitsyn, dholmes, rehn
>
> The following change in `src/hotspot/share/runtime/thread.cpp` added new assert:
>
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no \
> suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
> + "missing ThreadsListHandle in calling context.");
> return this->handshake_state()->suspend();
> }
>
> This new assert misses a check for target thread as being current `JavaThread`.
>
> Also, the JVMTI SuspendThread is protected with TLH:
>
> JvmtiEnv::SuspendThread(jthread thread) {
> JavaThread* current = JavaThread::current();
> ThreadsListHandle tlh(current); <= TLS defined here!!!
>
> oop thread_oop = NULL;
> {
> JvmtiVTMSTransitionDisabler disabler(true);
>
>
> However, it is possible that a new carrier thread (and an associated `JavaThread`) \
> can be created after the `TLH` was set and the target virtual thread can be mounted \
> on new carrier thread. Then target virtual thread will be associated with newly \
> created `JavaThread` which is unprotected by the TLH. The right way to be protected \
> from this situation it is to prevent mount state transitions with \
> `JvmtiVTMSTransitionDisabler` before the TLH is set as in the change below:
>
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, jthread** \
> threads_ptr) { jvmtiError
> JvmtiEnv::SuspendThread(jthread thread) {
> JavaThread* current = JavaThread::current();
> - ThreadsListHandle tlh(current);
>
> jvmtiError err;
> JavaThread* java_thread = NULL;
> oop thread_oop = NULL;
> {
> JvmtiVTMSTransitionDisabler disabler(true);
> + ThreadsListHandle tlh(current);
>
> err = get_threadOop_and_JavaThread(tlh.list(), thread, &java_thread, &thread_oop);
> if (err != JVMTI_ERROR_NONE) {
>
>
>
> This problem exist in all JVMTI Suspend functions:
> `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.
This pull request has now been integrated.
Changeset: 94811c0d
Author: Serguei Spitsyn <sspitsyn@openjdk.org>
URL: https://git.openjdk.java.net/jdk/commit/94811c0dc7c20b0e7cb2649fe8da5061ce3d6246
Stats: 17 lines in 2 files changed: 8 ins; 7 del; 2 mod
8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing \
ThreadsListHandle in calling context
Reviewed-by: dholmes, pchilanomate, amenkov
-------------
PR: https://git.openjdk.java.net/jdk/pull/8878
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic