8373839: Disable JVM TI suspension during JNI critical regions

Co-authored-by: Patricio Chilano Mateo <pchilanomate@openjdk.org>
Reviewed-by: sspitsyn, pchilanomate
This commit is contained in:
David Holmes 2026-05-11 04:49:11 +00:00
parent 5caa357fc6
commit 840154df43
9 changed files with 441 additions and 13 deletions

View File

@ -2835,6 +2835,10 @@ JNI_ENTRY(void*, jni_GetPrimitiveArrayCritical(JNIEnv *env, jarray array, jboole
Handle a(thread, JNIHandles::resolve_non_null(array));
assert(a->is_typeArray(), "just checking");
// We must defer JVM TI suspension while we have access to a Java object
// as it could surprise the debugger if we mutate it concurrently whilst
// logically suspended.
thread->enter_jni_deferred_suspension();
// Pin object
Universe::heap()->pin_object(thread, a());
@ -2852,6 +2856,7 @@ JNI_ENTRY(void, jni_ReleasePrimitiveArrayCritical(JNIEnv *env, jarray array, voi
HOTSPOT_JNI_RELEASEPRIMITIVEARRAYCRITICAL_ENTRY(env, array, carray, mode);
// Unpin object
Universe::heap()->unpin_object(thread, JNIHandles::resolve_non_null(array));
thread->exit_jni_deferred_suspension();
HOTSPOT_JNI_RELEASEPRIMITIVEARRAYCRITICAL_RETURN();
JNI_END
@ -2859,6 +2864,13 @@ JNI_END
JNI_ENTRY(const jchar*, jni_GetStringCritical(JNIEnv *env, jstring string, jboolean *isCopy))
HOTSPOT_JNI_GETSTRINGCRITICAL_ENTRY(env, string, (uintptr_t *) isCopy);
oop s = JNIHandles::resolve_non_null(string);
// We must defer JVM TI suspension while we have access to a Java object.
// Even if we are taking a private copy we must not be considered
// suspended as the debugger could be mutating the string we are about
// to copy.
thread->enter_jni_deferred_suspension();
jchar* ret;
if (!java_lang_String::is_latin1(s)) {
typeArrayHandle s_value(thread, java_lang_String::value(s));
@ -2879,6 +2891,10 @@ JNI_ENTRY(const jchar*, jni_GetStringCritical(JNIEnv *env, jstring string, jbool
ret[i] = ((jchar) s_value->byte_at(i)) & 0xff;
}
ret[s_len] = 0;
} else {
// If we return null there should not be a paired release operation
// so we have to cancel suspension deferral here.
thread->exit_jni_deferred_suspension();
}
if (isCopy != nullptr) *isCopy = JNI_TRUE;
}
@ -2904,6 +2920,7 @@ JNI_ENTRY(void, jni_ReleaseStringCritical(JNIEnv *env, jstring str, const jchar
// Unpin value array
Universe::heap()->unpin_object(thread, value);
}
thread->exit_jni_deferred_suspension();
HOTSPOT_JNI_RELEASESTRINGCRITICAL_RETURN();
JNI_END

View File

@ -83,8 +83,10 @@ class HandshakeOperation : public CHeapObj<mtThread> {
int32_t pending_threads() { return AtomicAccess::load(&_pending_threads); }
const char* name() { return _handshake_cl->name(); }
bool is_async() { return _handshake_cl->is_async(); }
bool is_suspend() { return _handshake_cl->is_suspend(); }
bool is_self_suspend() { return _handshake_cl->is_self_suspend(); }
bool is_suspend_request() { return _handshake_cl->is_suspend_request(); }
bool is_async_exception() { return _handshake_cl->is_async_exception(); }
bool is_enabled() { return _handshake_cl->is_enabled(_target); }
};
class AsyncHandshakeOperation : public HandshakeOperation {
@ -472,18 +474,24 @@ void Handshake::execute(AsyncHandshakeClosure* hs_cl, JavaThread* target) {
}
// Filters
// op is enabled and can be executed by the current thread rather than the target.
static bool non_self_executable_filter(HandshakeOperation* op) {
return !op->is_async();
return !op->is_async() && op->is_enabled();
}
// op is not an async-exception op
static bool no_async_exception_filter(HandshakeOperation* op) {
return !op->is_async_exception();
}
// op is an async-exception op
static bool async_exception_filter(HandshakeOperation* op) {
return op->is_async_exception();
}
// op is not any kind of suspend op, nor an async-exception op
static bool no_suspend_no_async_exception_filter(HandshakeOperation* op) {
return !op->is_suspend() && !op->is_async_exception();
return !op->is_self_suspend() && !op->is_suspend_request() && !op->is_async_exception();
}
// All ops
static bool all_ops_filter(HandshakeOperation* op) {
return true;
}
@ -521,8 +529,12 @@ HandshakeOperation* HandshakeState::get_op_for_self(bool allow_suspend, bool che
assert(_lock.owned_by_self(), "Lock must be held");
assert(allow_suspend || !check_async_exception, "invalid case");
#if INCLUDE_JVMTI
if (allow_suspend && (_handshakee->is_disable_suspend() || _handshakee->is_vthread_transition_disabler())) {
// filter out suspend operations while JavaThread can not be suspended
// Filter out suspend operations while JavaThread can not be suspended.
// Potentially this could be folded into the `is_enabled` state of the operation
// and filtered directly through _queue.peek, but the incoming `allow_suspend`
// complicates that so we just maintain the explicit checks for now.
if (allow_suspend && (_handshakee->is_disable_suspend() || _handshakee->is_vthread_transition_disabler() ||
_handshakee->jni_deferred_suspension())) {
allow_suspend = false;
}
#endif
@ -565,12 +577,16 @@ void HandshakeState::clean_async_exception_operation() {
}
}
// Returns true if there is an enabled op that the current thread can execute
// on behalf of the handshakee.
bool HandshakeState::have_non_self_executable_operation() {
assert(_handshakee != Thread::current(), "Must not be called by self");
assert(_lock.owned_by_self(), "Lock must be held");
return _queue.contains(non_self_executable_filter);
}
// Returns an enabled op that the current thread can execute
// on behalf of the handshakee.
HandshakeOperation* HandshakeState::get_op() {
assert(_handshakee != Thread::current(), "Must not be called by self");
assert(_lock.owned_by_self(), "Lock must be held");
@ -683,7 +699,7 @@ HandshakeState::ProcessResult HandshakeState::try_process(HandshakeOperation* ma
return HandshakeState::_not_safe;
}
// Claim the mutex if there still an operation to be executed.
// Claim the mutex if there still an enabled operation to be executed.
if (!claim_handshake()) {
return HandshakeState::_claim_failed;
}
@ -699,8 +715,13 @@ HandshakeState::ProcessResult HandshakeState::try_process(HandshakeOperation* ma
Thread* current_thread = Thread::current();
HandshakeOperation* op = get_op();
// It is possible that since we claimed the handshake the op has
// transitioned to a disabled state and so won't be returned by get_op.
if (op == nullptr) {
return HandshakeState::_no_operation;
}
assert(op != nullptr, "Must have an op");
assert(op->is_enabled(), "Should not reach here with a disabled op");
assert(SafepointMechanism::local_poll_armed(_handshakee), "Must be");
assert(op->_target == nullptr || _handshakee == op->_target, "Wrong thread");

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -50,8 +50,10 @@ class HandshakeClosure : public ThreadClosure, public CHeapObj<mtThread> {
virtual ~HandshakeClosure() {}
const char* name() const { return _name; }
virtual bool is_async() { return false; }
virtual bool is_suspend() { return false; }
virtual bool is_self_suspend() { return false; }
virtual bool is_suspend_request() { return false; }
virtual bool is_async_exception() { return false; }
virtual bool is_enabled(Thread* target) { return true; }
virtual void do_thread(Thread* thread) = 0;
};
@ -99,7 +101,6 @@ class HandshakeState {
Monitor _lock;
// Set to the thread executing the handshake operation.
Thread* volatile _active_handshaker;
bool claim_handshake();
bool possibly_can_process_handshake();
bool can_process_handshake();

View File

@ -469,6 +469,7 @@ JavaThread::JavaThread(MemTag mem_tag) :
_exception_handler_pc(nullptr),
_jni_active_critical(0),
_jni_deferred_suspension_count(0),
_pending_jni_exception_check_fn(nullptr),
_depth_first_number(0),

View File

@ -458,9 +458,12 @@ class JavaThread: public Thread {
volatile address _exception_handler_pc; // PC for handler of exception
private:
// support for JNI critical regions
// support for JNI critical regions interaction with GC
jint _jni_active_critical; // count of entries into JNI critical region
// support for JNI critical regions deferral of JVM TI suspension
jint _jni_deferred_suspension_count;
// Checked JNI: function name requires exception check
char* _pending_jni_exception_check_fn;
@ -980,10 +983,19 @@ public:
_jni_active_critical--;
assert(_jni_active_critical >= 0, "JNI critical nesting problem?");
}
// Atomic version; invoked by a thread other than the owning thread.
bool in_critical_atomic() { return AtomicAccess::load(&_jni_active_critical) > 0; }
bool jni_deferred_suspension() { return AtomicAccess::load(&_jni_deferred_suspension_count); }
inline void enter_jni_deferred_suspension();
void exit_jni_deferred_suspension() {
precond(Thread::current() == this);
int sc = AtomicAccess::load(&_jni_deferred_suspension_count);
AtomicAccess::store(&_jni_deferred_suspension_count, sc - 1);
assert(_jni_deferred_suspension_count >= 0,
"JNI deferred suspension nesting problem?");
}
// Checked JNI: is the programmer required to check for exceptions, if so specify
// which function name. Returning to a Java frame should implicitly clear the
// pending check, this is done for Native->Java transitions (i.e. user JNI code).

View File

@ -196,6 +196,14 @@ void JavaThread::enter_critical() {
_jni_active_critical++;
}
void JavaThread::enter_jni_deferred_suspension() {
precond(JavaThread::current() == this);
assert(_thread_state != _thread_in_native && _thread_state != _thread_blocked,
"Must not defer suspension when handshake-safe");
int sc = AtomicAccess::load(&_jni_deferred_suspension_count);
AtomicAccess::store(&_jni_deferred_suspension_count, sc + 1);
}
inline void JavaThread::set_done_attaching_via_jni() {
_jni_attach_state = _attached_via_jni;
OrderAccess::fence();

View File

@ -48,7 +48,7 @@ public:
current->set_thread_state(jts);
current->suspend_resume_manager()->set_async_suspend_handshake(false);
}
virtual bool is_suspend() { return true; }
virtual bool is_self_suspend() { return true; }
};
// This is the closure that synchronously honors the suspend request.
@ -64,6 +64,16 @@ public:
_did_suspend = target->suspend_resume_manager()->suspend_with_handshake(_register_vthread_SR);
}
bool did_suspend() { return _did_suspend; }
virtual bool is_suspend_request() { return true; }
// If the target is in a JNI deferred suspension region, then we cannot
// process this operation. This must be checked with the HandshakeState lock
// held, which together with the fact the target only enters a deferred
// region from a handshake-unsafe state, means we cannot race with the
// target entering that region.
virtual bool is_enabled(Thread* target) {
return !JavaThread::cast(target)->jni_deferred_suspension();
}
};
void SuspendResumeManager::set_suspended(bool is_suspend, bool register_vthread_SR) {

View File

@ -0,0 +1,212 @@
/*
* Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/**
* @test
* @bug 8373839
* @requires vm.jvmti
* @library /test/lib
* @run main/othervm/native -agentlib:SuspendInCritical SuspendInCritical 1
* @run main/othervm/native -agentlib:SuspendInCritical SuspendInCritical 2
* @run main/othervm/native -agentlib:SuspendInCritical SuspendInCritical 3
*/
import jdk.test.lib.Asserts;
import java.util.concurrent.CountDownLatch;
public class SuspendInCritical {
static {
System.loadLibrary("SuspendInCritical");
}
static native void suspendThread(Thread t);
static native void resumeThread(Thread t);
static native boolean isSuspended(Thread t);
static native void doCritical(byte[] b, String str);
static native void leaveCriticalNative();
static native long getNativeCounter();
static volatile boolean suspendStarted = false;
static volatile boolean suspendCompleted = false;
static volatile boolean canTerminate = false;
/*
* Incoming arg sets the mode:
* - 1: GetPrimitiveArrayCritical only
* - 2: GetStringCritical only
* - 3: Do both so we have a nested critical region
*/
public static void main(String[] args) throws Throwable {
int mode = Integer.parseInt(args[0]);
final byte[] bytes = (mode == 1 || mode == 3) ? new byte[16] : null;
final String str = (mode == 2 || mode == 3) ? "A String" : null;
final Thread t = new Thread() {
public void run() {
System.out.println("CriticalThread calling native ...");
doCritical(bytes, str);
System.out.println("CriticalThread return from native ...");
// delay termination so we can check the suspend state
// from another thread
while (!canTerminate) {
delay(10);
}
System.out.println("CriticalThread terminating");
}
};
t.setName("CriticalThread");
System.out.println("main thread starting CriticalThread");
// Start the target thread. It will enter a JNI critical region
// and stay executing native code until released.
t.start();
// Check that the counter is progressing
checkNativeCounter();
System.out.println("main thread saw CriticalThread executing and is starting SuspenderThread");
// Now start the suspender thread.
Thread s = new Thread() {
public void run() {
suspendStarted = true;
System.out.println("SuspenderThread calling suspend ...");
// This will block until t is out of the critical region
suspendThread(t);
suspendCompleted = true;
System.out.println("SuspenderThread checking suspend ...");
// Verify t is suspended
Asserts.assertTrue(isSuspended(t), "not suspended");
System.out.println("SuspenderThread calling resume ...");
// Resume t
resumeThread(t);
System.out.println("SuspenderThread checking not suspended ...");
Asserts.assertFalse(isSuspended(t), "suspended");
System.out.println("SuspenderThread allowing target to terminate ...");
// Allow t to terminate
canTerminate = true;
System.out.println("SuspenderThread terminatng");
}
};
s.setName("SuspenderThread");
s.start();
while (!suspendStarted) {
delay(2);
}
// Check suspender is blocked
checkSuspenderIsBlocked();
System.out.println("main thread confirms SuspenderThread is blocked in suspend()");
// Check target is still progressing
checkNativeCounter();
System.out.println("main thread saw CriticalThread still executing and has enabled the upcall");
// Allow target to proceed to Java upcall
leaveCriticalNative();
// Wait till target is in Java upcall
waitForUpcall();
System.out.println("main thread saw CriticalThread in upcall");
// Check suspender is blocked
checkSuspenderIsBlocked();
System.out.println("main thread confirms SuspenderThread is still blocked in suspend()");
// Check target is still executing
checkUpcallCount();
System.out.println("main thread confirms CriticalThread is still executing and will let it return and be suspended");
// Check suspender is still blocked
checkSuspenderIsBlocked();
// Allow target to return from Java and exit critical
upcallDone = true;
// The suspension can now proceed, then both threads will terminate
t.join();
s.join();
System.out.println("main thread terminating");
}
static void checkNativeCounter() {
long counter = getNativeCounter();
// If the counter never progresses then the test will timeout
while (counter == getNativeCounter()) {
delay(5);
}
}
static void checkSuspenderIsBlocked() {
delay(200);
Asserts.assertTrue(!suspendCompleted,"Unexpected suspend completion");
}
static volatile boolean upcallDone = false;
static volatile long upcallCounter = 0;
static CountDownLatch inUpcall = new CountDownLatch(1);
static void waitForUpcall() throws InterruptedException {
inUpcall.await();
}
static void upcall() {
inUpcall.countDown();
System.out.println("CriticalThread is executing upcall");
while (!upcallDone) {
upcallCounter++;
delay(1);
}
}
static void checkUpcallCount() {
long count = upcallCounter;
// If the counter never progresses then the test will timeout
while (count == upcallCounter) {
delay(5);
}
}
static void delay(long ms) {
try {
Thread.sleep(ms);
}
catch (InterruptedException ex) {
throw new Error("interrupted!");
}
}
}

View File

@ -0,0 +1,146 @@
/*
* Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#include <atomic>
#include <jni.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "jvmti.h"
#include "jvmti_common.hpp"
static jvmtiEnv* jvmti = nullptr;
static std::atomic<jboolean> stay_in_critical_native{JNI_TRUE};
static std::atomic<jlong> native_counter{0};
extern "C" {
JNIEXPORT void JNICALL
Java_SuspendInCritical_suspendThread(JNIEnv* jni, jclass cls, jthread t) {
suspend_thread(jvmti, jni, t);
}
JNIEXPORT void JNICALL
Java_SuspendInCritical_resumeThread(JNIEnv* jni, jclass cls, jthread t) {
resume_thread(jvmti, jni, t);
}
JNIEXPORT jboolean JNICALL
Java_SuspendInCritical_isSuspended(JNIEnv* jni, jclass cls, jthread t) {
jint state = get_thread_state(jvmti, jni, t);
return (state & JVMTI_THREAD_STATE_SUSPENDED) != 0;
}
JNIEXPORT void JNICALL
Java_SuspendInCritical_leaveCriticalNative(JNIEnv* jni, jclass cls) {
stay_in_critical_native = JNI_FALSE;
}
JNIEXPORT jlong JNICALL
Java_SuspendInCritical_getNativeCounter(JNIEnv* jni, jclass cls) {
return native_counter;
}
JNIEXPORT void JNICALL
Java_SuspendInCritical_doCritical(JNIEnv* jni, jclass cls, jbyteArray bytes, jstring str) {
jboolean is_copy = JNI_FALSE;
jbyte* b;
if (bytes != nullptr) {
LOG("CriticalThread doing GetPrimitiveArrayCritical\n");
b = (jbyte*) jni->GetPrimitiveArrayCritical(bytes, &is_copy);
if (b == nullptr) {
jni->FatalError("GetPrimitiveArrayCritical returned null!");
}
}
const jchar* c;
if (str != nullptr) {
LOG("CriticalThread doing GetStringCritical\n");
c = jni->GetStringCritical(str, &is_copy);
if (c == nullptr) {
jni->FatalError("GetStringCritical returned null!");
}
}
LOG("CriticalThread should now be in deferred suspension\n");
// Do some visible work
while (stay_in_critical_native) {
native_counter++;
sleep_ms(1);
}
LOG("CriticalThread released for Java upcall\n");
// Now perform the Java upcall
jmethodID upcall_mid = jni->GetStaticMethodID(cls, "upcall", "()V");
if (upcall_mid == nullptr) {
// Unexpected exception - let it propagate
if (bytes != nullptr) {
jni->ReleasePrimitiveArrayCritical(bytes, b, 0);
}
if (str != nullptr) {
jni->ReleaseStringCritical(str, c);
}
return;
}
jni->CallStaticVoidMethod(cls, upcall_mid);
if (bytes != nullptr) {
jni->ReleasePrimitiveArrayCritical(bytes, b, 0);
}
if (str != nullptr) {
jni->ReleaseStringCritical(str, c);
}
// Now we should suspend as we return to Java
LOG("CriticalThread returning to Java\n");
}
/** Agent library initialization. */
JNIEXPORT jint JNICALL
Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) {
LOG("\nAgent_OnLoad started\n");
// create JVMTI environment
if (jvm->GetEnv((void **) (&jvmti), JVMTI_VERSION_1_1) != JNI_OK) {
LOG("Wrong result of a valid call to GetEnv!\n");
return JNI_ERR;
}
// add specific capabilities for suspending thread
jvmtiCapabilities suspendCaps;
memset(&suspendCaps, 0, sizeof(suspendCaps));
suspendCaps.can_suspend = 1;
jvmtiError err = jvmti->AddCapabilities(&suspendCaps);
if (err != JVMTI_ERROR_NONE) {
LOG("(AddCapabilities) unexpected error: %s (%d)\n", TranslateError(err), err);
return JNI_ERR;
}
LOG("Agent_OnLoad finished\n");
return JNI_OK;
}
} // extern C