Skip to content

Commit dc06fed

Browse files
author
Serguei Spitsyn
committed
8373367: interp-only mechanism fails to work for carrier threads in a corner case
Reviewed-by: amenkov, lmesnik
1 parent 6daca7e commit dc06fed

File tree

7 files changed

+61
-52
lines changed

7 files changed

+61
-52
lines changed

src/hotspot/share/prims/jvmtiEnvBase.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -2491,7 +2491,7 @@ SetOrClearFramePopClosure::do_thread(Thread *target) {
24912491
_result = JVMTI_ERROR_NO_MORE_FRAMES;
24922492
return;
24932493
}
2494-
assert(_state->get_thread_or_saved() == java_thread, "Must be");
2494+
assert(_state->get_thread() == java_thread, "Must be");
24952495

24962496
RegisterMap reg_map(java_thread,
24972497
RegisterMap::UpdateMap::include,

src/hotspot/share/prims/jvmtiEnvThreadState.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -151,11 +151,6 @@ bool JvmtiEnvThreadState::is_virtual() {
151151
return _state->is_virtual();
152152
}
153153

154-
// Use _thread_saved if cthread is detached from JavaThread (_thread == nullptr).
155-
JavaThread* JvmtiEnvThreadState::get_thread_or_saved() {
156-
return _state->get_thread_or_saved();
157-
}
158-
159154
JavaThread* JvmtiEnvThreadState::get_thread() {
160155
return _state->get_thread();
161156
}
@@ -344,7 +339,7 @@ void JvmtiEnvThreadState::reset_current_location(jvmtiEvent event_type, bool ena
344339
if (enabled) {
345340
// If enabling breakpoint, no need to reset.
346341
// Can't do anything if empty stack.
347-
JavaThread* thread = get_thread_or_saved();
342+
JavaThread* thread = get_thread();
348343

349344
if (event_type == JVMTI_EVENT_SINGLE_STEP &&
350345
((thread == nullptr && is_virtual()) || thread->has_last_Java_frame())) {

src/hotspot/share/prims/jvmtiEnvThreadState.hpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -170,8 +170,6 @@ class JvmtiEnvThreadState : public CHeapObj<mtInternal> {
170170

171171
inline JvmtiThreadState* jvmti_thread_state() { return _state; }
172172

173-
// use _thread_saved if cthread is detached from JavaThread
174-
JavaThread *get_thread_or_saved();
175173
JavaThread *get_thread();
176174
inline JvmtiEnv *get_env() { return _env; }
177175

src/hotspot/share/prims/jvmtiEventController.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -217,6 +217,10 @@ class EnterInterpOnlyModeClosure : public HandshakeClosure {
217217

218218
assert(state != nullptr, "sanity check");
219219
assert(state->get_thread() == jt, "handshake unsafe conditions");
220+
assert(jt->jvmti_thread_state() == state, "sanity check");
221+
assert(!jt->is_interp_only_mode(), "sanity check");
222+
assert(!state->is_interp_only_mode(), "sanity check");
223+
220224
if (!state->is_pending_interp_only_mode()) {
221225
_completed = true;
222226
return; // The pending flag has been already cleared, so bail out.
@@ -361,7 +365,8 @@ void VM_ChangeSingleStep::doit() {
361365

362366
void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state) {
363367
EC_TRACE(("[%s] # Entering interpreter only mode",
364-
JvmtiTrace::safe_get_thread_name(state->get_thread_or_saved())));
368+
JvmtiTrace::safe_get_thread_name(state->get_thread())));
369+
365370
JavaThread *target = state->get_thread();
366371
Thread *current = Thread::current();
367372

@@ -371,8 +376,13 @@ void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state
371376
}
372377
// This flag will be cleared in EnterInterpOnlyModeClosure handshake.
373378
state->set_pending_interp_only_mode(true);
374-
if (target == nullptr) { // an unmounted virtual thread
375-
return; // EnterInterpOnlyModeClosure will be executed right after mount.
379+
380+
// There are two cases when entering interp_only_mode is postponed:
381+
// 1. Unmounted virtual thread - EnterInterpOnlyModeClosure::do_thread will be executed at mount;
382+
// 2. Carrier thread with mounted virtual thread - EnterInterpOnlyModeClosure::do_thread will be executed at unmount.
383+
if (target == nullptr || // an unmounted virtual thread
384+
JvmtiEnvBase::is_thread_carrying_vthread(target, state->get_thread_oop())) { // a vthread carrying thread
385+
return; // EnterInterpOnlyModeClosure will be executed right after mount or unmount.
376386
}
377387
EnterInterpOnlyModeClosure hs(state);
378388
if (target->is_handshake_safe_for(current)) {
@@ -388,7 +398,8 @@ void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state
388398
void
389399
JvmtiEventControllerPrivate::leave_interp_only_mode(JvmtiThreadState *state) {
390400
EC_TRACE(("[%s] # Leaving interpreter only mode",
391-
JvmtiTrace::safe_get_thread_name(state->get_thread_or_saved())));
401+
JvmtiTrace::safe_get_thread_name(state->get_thread())));
402+
392403
if (state->is_pending_interp_only_mode()) {
393404
state->set_pending_interp_only_mode(false); // Just clear the pending flag.
394405
assert(!state->is_interp_only_mode(), "sanity check");
@@ -409,7 +420,7 @@ JvmtiEventControllerPrivate::trace_changed(JvmtiThreadState *state, jlong now_en
409420
if (changed & bit) {
410421
// it changed, print it
411422
log_trace(jvmti)("[%s] # %s event %s",
412-
JvmtiTrace::safe_get_thread_name(state->get_thread_or_saved()),
423+
JvmtiTrace::safe_get_thread_name(state->get_thread()),
413424
(now_enabled & bit)? "Enabling" : "Disabling", JvmtiTrace::event_name((jvmtiEvent)ei));
414425
}
415426
}
@@ -932,7 +943,7 @@ JvmtiEventControllerPrivate::set_user_enabled(JvmtiEnvBase *env, JavaThread *thr
932943
void
933944
JvmtiEventControllerPrivate::set_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) {
934945
EC_TRACE(("[%s] # set frame pop - frame=%d",
935-
JvmtiTrace::safe_get_thread_name(ets->get_thread_or_saved()),
946+
JvmtiTrace::safe_get_thread_name(ets->get_thread()),
936947
fpop.frame_number() ));
937948

938949
ets->get_frame_pops()->set(fpop);
@@ -943,7 +954,7 @@ JvmtiEventControllerPrivate::set_frame_pop(JvmtiEnvThreadState *ets, JvmtiFrameP
943954
void
944955
JvmtiEventControllerPrivate::clear_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) {
945956
EC_TRACE(("[%s] # clear frame pop - frame=%d",
946-
JvmtiTrace::safe_get_thread_name(ets->get_thread_or_saved()),
957+
JvmtiTrace::safe_get_thread_name(ets->get_thread()),
947958
fpop.frame_number() ));
948959

949960
ets->get_frame_pops()->clear(fpop);
@@ -953,7 +964,7 @@ JvmtiEventControllerPrivate::clear_frame_pop(JvmtiEnvThreadState *ets, JvmtiFram
953964
void
954965
JvmtiEventControllerPrivate::clear_all_frame_pops(JvmtiEnvThreadState *ets) {
955966
EC_TRACE(("[%s] # clear all frame pops",
956-
JvmtiTrace::safe_get_thread_name(ets->get_thread_or_saved())
967+
JvmtiTrace::safe_get_thread_name(ets->get_thread())
957968
));
958969

959970
ets->get_frame_pops()->clear_all();
@@ -965,7 +976,7 @@ JvmtiEventControllerPrivate::clear_to_frame_pop(JvmtiEnvThreadState *ets, JvmtiF
965976
int cleared_cnt = ets->get_frame_pops()->clear_to(fpop);
966977

967978
EC_TRACE(("[%s] # clear to frame pop - frame=%d, count=%d",
968-
JvmtiTrace::safe_get_thread_name(ets->get_thread_or_saved()),
979+
JvmtiTrace::safe_get_thread_name(ets->get_thread()),
969980
fpop.frame_number(),
970981
cleared_cnt ));
971982

src/hotspot/share/prims/jvmtiThreadState.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -57,7 +57,6 @@ JvmtiThreadState::JvmtiThreadState(JavaThread* thread, oop thread_oop)
5757
: _thread_event_enable() {
5858
assert(JvmtiThreadState_lock->is_locked(), "sanity check");
5959
_thread = thread;
60-
_thread_saved = nullptr;
6160
_exception_state = ES_CLEARED;
6261
_hide_single_stepping = false;
6362
_pending_interp_only_mode = false;
@@ -118,11 +117,11 @@ JvmtiThreadState::JvmtiThreadState(JavaThread* thread, oop thread_oop)
118117

119118
if (thread != nullptr) {
120119
if (thread_oop == nullptr || thread->jvmti_vthread() == nullptr || thread->jvmti_vthread() == thread_oop) {
121-
// The JavaThread for carrier or mounted virtual thread case.
120+
// The JavaThread for an active carrier or a mounted virtual thread case.
122121
// Set this only if thread_oop is current thread->jvmti_vthread().
123122
thread->set_jvmti_thread_state(this);
123+
assert(!thread->is_interp_only_mode(), "sanity check");
124124
}
125-
thread->set_interp_only_mode(false);
126125
}
127126
}
128127

@@ -135,7 +134,10 @@ JvmtiThreadState::~JvmtiThreadState() {
135134
}
136135

137136
// clear this as the state for the thread
137+
assert(get_thread() != nullptr, "sanity check");
138+
assert(get_thread()->jvmti_thread_state() == this, "sanity check");
138139
get_thread()->set_jvmti_thread_state(nullptr);
140+
get_thread()->set_interp_only_mode(false);
139141

140142
// zap our env thread states
141143
{
@@ -323,25 +325,27 @@ void JvmtiThreadState::enter_interp_only_mode() {
323325
assert(_thread != nullptr, "sanity check");
324326
assert(JvmtiThreadState_lock->is_locked(), "sanity check");
325327
assert(!is_interp_only_mode(), "entering interp only when in interp only mode");
328+
assert(_thread->jvmti_vthread() == nullptr || _thread->jvmti_vthread() == get_thread_oop(), "sanity check");
329+
assert(_thread->jvmti_thread_state() == this, "sanity check");
330+
_saved_interp_only_mode = true;
326331
_thread->set_interp_only_mode(true);
327332
invalidate_cur_stack_depth();
328333
}
329334

330335
void JvmtiThreadState::leave_interp_only_mode() {
331336
assert(JvmtiThreadState_lock->is_locked(), "sanity check");
332337
assert(is_interp_only_mode(), "leaving interp only when not in interp only mode");
333-
if (_thread == nullptr) {
334-
// Unmounted virtual thread updates the saved value.
335-
_saved_interp_only_mode = false;
336-
} else {
338+
_saved_interp_only_mode = false;
339+
if (_thread != nullptr && _thread->jvmti_thread_state() == this) {
340+
assert(_thread->jvmti_vthread() == nullptr || _thread->jvmti_vthread() == get_thread_oop(), "sanity check");
337341
_thread->set_interp_only_mode(false);
338342
}
339343
}
340344

341345

342346
// Helper routine used in several places
343347
int JvmtiThreadState::count_frames() {
344-
JavaThread* thread = get_thread_or_saved();
348+
JavaThread* thread = get_thread();
345349
javaVFrame *jvf;
346350
ResourceMark rm;
347351
if (thread == nullptr) {
@@ -578,11 +582,8 @@ void JvmtiThreadState::update_thread_oop_during_vm_start() {
578582
}
579583
}
580584

585+
// For virtual threads only.
581586
void JvmtiThreadState::set_thread(JavaThread* thread) {
582-
_thread_saved = nullptr; // Common case.
583-
if (!_is_virtual && thread == nullptr) {
584-
// Save JavaThread* if carrier thread is being detached.
585-
_thread_saved = _thread;
586-
}
587+
assert(is_virtual(), "sanity check");
587588
_thread = thread;
588589
}

src/hotspot/share/prims/jvmtiThreadState.hpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -123,8 +123,11 @@ class JvmtiVTSuspender : AllStatic {
123123
class JvmtiThreadState : public CHeapObj<mtInternal> {
124124
private:
125125
friend class JvmtiEnv;
126+
// The _thread field is a link to the JavaThread associated with JvmtiThreadState.
127+
// A platform (including carrier) thread should always have a stable link to its JavaThread.
128+
// The _thread field of a virtual thread should point to the JavaThread when
129+
// virtual thread is mounted. It should be set to null when it is unmounted.
126130
JavaThread *_thread;
127-
JavaThread *_thread_saved;
128131
OopHandle _thread_oop_h;
129132
// Jvmti Events that cannot be posted in their current context.
130133
JvmtiDeferredEventQueue* _jvmti_event_queue;
@@ -219,7 +222,7 @@ class JvmtiThreadState : public CHeapObj<mtInternal> {
219222

220223
// Used by the interpreter for fullspeed debugging support
221224
bool is_interp_only_mode() {
222-
return _thread == nullptr ? _saved_interp_only_mode : _thread->is_interp_only_mode();
225+
return _saved_interp_only_mode;
223226
}
224227
void enter_interp_only_mode();
225228
void leave_interp_only_mode();
@@ -248,8 +251,10 @@ class JvmtiThreadState : public CHeapObj<mtInternal> {
248251

249252
int count_frames();
250253

251-
inline JavaThread *get_thread() { return _thread; }
252-
inline JavaThread *get_thread_or_saved(); // return _thread_saved if _thread is null
254+
inline JavaThread *get_thread() {
255+
assert(is_virtual() || _thread != nullptr, "sanity check");
256+
return _thread;
257+
}
253258

254259
// Needed for virtual threads as they can migrate to different JavaThread's.
255260
// Also used for carrier threads to clear/restore _thread.

src/hotspot/share/prims/jvmtiThreadState.inline.hpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2006, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2006, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -130,22 +130,21 @@ inline JvmtiThreadState* JvmtiThreadState::state_for(JavaThread *thread, Handle
130130
return state;
131131
}
132132

133-
inline JavaThread* JvmtiThreadState::get_thread_or_saved() {
134-
// Use _thread_saved if cthread is detached from JavaThread (_thread == null).
135-
return (_thread == nullptr && !is_virtual()) ? _thread_saved : _thread;
136-
}
137-
138133
inline void JvmtiThreadState::set_should_post_on_exceptions(bool val) {
139-
get_thread_or_saved()->set_should_post_on_exceptions_flag(val ? JNI_TRUE : JNI_FALSE);
134+
get_thread()->set_should_post_on_exceptions_flag(val ? JNI_TRUE : JNI_FALSE);
140135
}
141136

142137
inline void JvmtiThreadState::unbind_from(JvmtiThreadState* state, JavaThread* thread) {
143138
if (state == nullptr) {
139+
assert(!thread->is_interp_only_mode(), "sanity check");
144140
return;
145141
}
146-
// Save thread's interp_only_mode.
147-
state->_saved_interp_only_mode = thread->is_interp_only_mode();
148-
state->set_thread(nullptr); // Make sure stale _thread value is never used.
142+
assert(thread->jvmti_thread_state() == state, "sanity check");
143+
assert(state->get_thread() == thread, "sanity check");
144+
assert(thread->is_interp_only_mode() == state->_saved_interp_only_mode, "sanity check");
145+
if (state->is_virtual()) { // clean _thread link for virtual threads only
146+
state->set_thread(nullptr); // make sure stale _thread value is never used
147+
}
149148
}
150149

151150
inline void JvmtiThreadState::bind_to(JvmtiThreadState* state, JavaThread* thread) {
@@ -158,7 +157,7 @@ inline void JvmtiThreadState::bind_to(JvmtiThreadState* state, JavaThread* threa
158157
// Bind JavaThread to JvmtiThreadState.
159158
thread->set_jvmti_thread_state(state);
160159

161-
if (state != nullptr) {
160+
if (state != nullptr && state->is_virtual()) {
162161
// Bind to JavaThread.
163162
state->set_thread(thread);
164163
}

0 commit comments

Comments
 (0)