From 1e6770fb978e630b38a70a05120c50f723bb66dc Mon Sep 17 00:00:00 2001 From: David Holmes Date: Tue, 30 May 2023 22:46:06 +0000 Subject: [PATCH] 8308341: JNI_GetCreatedJavaVMs returns a partially initialized JVM Reviewed-by: jsjolen, gziemski --- make/test/JtregNativeHotspot.gmk | 5 +- src/hotspot/share/prims/jni.cpp | 40 ++++--- .../TestGetCreatedJavaVMs.java | 42 +++++++ .../getCreatedJavaVMs/exeGetCreatedJavaVMs.c | 104 ++++++++++++++++++ 4 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/TestGetCreatedJavaVMs.java create mode 100644 test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/exeGetCreatedJavaVMs.c diff --git a/make/test/JtregNativeHotspot.gmk b/make/test/JtregNativeHotspot.gmk index 1d58c72554d..bfc32960f64 100644 --- a/make/test/JtregNativeHotspot.gmk +++ b/make/test/JtregNativeHotspot.gmk @@ -1,5 +1,5 @@ # -# Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2015, 2023, 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 @@ -874,7 +874,7 @@ BUILD_HOTSPOT_JTREG_EXECUTABLES_LIBS_exesigtest := -ljvm ifeq ($(call isTargetOs, windows), true) BUILD_HOTSPOT_JTREG_EXECUTABLES_CFLAGS_exeFPRegs := -MT - BUILD_HOTSPOT_JTREG_EXCLUDE += exesigtest.c libterminatedThread.c libTestJNI.c libCompleteExit.c libTestPsig.c libnativeStack.c + BUILD_HOTSPOT_JTREG_EXCLUDE += exesigtest.c libterminatedThread.c libTestJNI.c libCompleteExit.c libTestPsig.c libnativeStack.c exeGetCreatedJavaVMs.c BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libatExit := jvm.lib BUILD_HOTSPOT_JTREG_EXECUTABLES_LIBS_exedaemonDestroy := jvm.lib else @@ -1516,6 +1516,7 @@ else BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libatExit += -ljvm BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libCompleteExit += -lpthread BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libnativeStack += -lpthread + BUILD_HOTSPOT_JTREG_EXECUTABLES_LIBS_exeGetCreatedJavaVMs := -ljvm -lpthread endif ifeq ($(ASAN_ENABLED), true) diff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp index 9588ae995f8..292660ae7c9 100644 --- a/src/hotspot/share/prims/jni.cpp +++ b/src/hotspot/share/prims/jni.cpp @@ -3472,7 +3472,14 @@ struct JNINativeInterface_* jni_functions_nocheck() { extern const struct JNIInvokeInterface_ jni_InvokeInterface; // Global invocation API vars -volatile int vm_created = 0; +enum VM_Creation_State { + NOT_CREATED = 0, + IN_PROGRESS, + COMPLETE +}; + +volatile VM_Creation_State vm_created = NOT_CREATED; + // Indicate whether it is safe to recreate VM. Recreation is only // possible after a failed initial creation attempt in some cases. volatile int safe_to_recreate_vm = 1; @@ -3541,7 +3548,7 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) { // We use Atomic::xchg rather than Atomic::add/dec since on some platforms // the add/dec implementations are dependent on whether we are running // on a multiprocessor Atomic::xchg does not have this problem. - if (Atomic::xchg(&vm_created, 1) == 1) { + if (Atomic::xchg(&vm_created, IN_PROGRESS) != NOT_CREATED) { return JNI_EEXIST; // already created, or create attempt in progress } @@ -3554,8 +3561,6 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) { return JNI_ERR; } - assert(vm_created == 1, "vm_created is true during the creation"); - /** * Certain errors during initialization are recoverable and do not * prevent this method from being called again at a later time @@ -3572,9 +3577,11 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) { if (result == JNI_OK) { JavaThread *thread = JavaThread::current(); assert(!thread->has_pending_exception(), "should have returned not OK"); - /* thread is thread_in_vm here */ + // thread is thread_in_vm here *vm = (JavaVM *)(&main_vm); *(JNIEnv**)penv = thread->jni_environment(); + // mark creation complete for other JNI ops + Atomic::release_store(&vm_created, COMPLETE); #if INCLUDE_JVMCI if (EnableJVMCI) { @@ -3637,7 +3644,8 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) { *(JNIEnv**)penv = 0; // reset vm_created last to avoid race condition. Use OrderAccess to // control both compiler and architectural-based reordering. - Atomic::release_store(&vm_created, 0); + assert(vm_created == IN_PROGRESS, "must be"); + Atomic::release_store(&vm_created, NOT_CREATED); } // Flush stdout and stderr before exit. @@ -3666,7 +3674,7 @@ _JNI_IMPORT_OR_EXPORT_ jint JNICALL JNI_CreateJavaVM(JavaVM **vm, void **penv, v _JNI_IMPORT_OR_EXPORT_ jint JNICALL JNI_GetCreatedJavaVMs(JavaVM **vm_buf, jsize bufLen, jsize *numVMs) { HOTSPOT_JNI_GETCREATEDJAVAVMS_ENTRY((void **) vm_buf, bufLen, (uintptr_t *) numVMs); - if (vm_created == 1) { + if (vm_created == COMPLETE) { if (numVMs != nullptr) *numVMs = 1; if (bufLen > 0) *vm_buf = (JavaVM *)(&main_vm); } else { @@ -3686,7 +3694,7 @@ static jint JNICALL jni_DestroyJavaVM_inner(JavaVM *vm) { jint res = JNI_ERR; DT_RETURN_MARK(DestroyJavaVM, jint, (const jint&)res); - if (vm_created == 0) { + if (vm_created != COMPLETE) { res = JNI_ERR; return res; } @@ -3717,7 +3725,7 @@ static jint JNICALL jni_DestroyJavaVM_inner(JavaVM *vm) { ThreadStateTransition::transition_from_native(thread, _thread_in_vm); Threads::destroy_vm(); // Don't bother restoring thread state, VM is gone. - vm_created = 0; + vm_created = NOT_CREATED; return JNI_OK; } @@ -3851,7 +3859,8 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae jint JNICALL jni_AttachCurrentThread(JavaVM *vm, void **penv, void *_args) { HOTSPOT_JNI_ATTACHCURRENTTHREAD_ENTRY(vm, penv, _args); - if (vm_created == 0) { + if (vm_created != COMPLETE) { + // Not sure how we could possibly get here. HOTSPOT_JNI_ATTACHCURRENTTHREAD_RETURN((uint32_t) JNI_ERR); return JNI_ERR; } @@ -3864,7 +3873,8 @@ jint JNICALL jni_AttachCurrentThread(JavaVM *vm, void **penv, void *_args) { jint JNICALL jni_DetachCurrentThread(JavaVM *vm) { HOTSPOT_JNI_DETACHCURRENTTHREAD_ENTRY(vm); - if (vm_created == 0) { + if (vm_created != COMPLETE) { + // Not sure how we could possibly get here. HOTSPOT_JNI_DETACHCURRENTTHREAD_RETURN(JNI_ERR); return JNI_ERR; } @@ -3927,7 +3937,10 @@ jint JNICALL jni_GetEnv(JavaVM *vm, void **penv, jint version) { jint ret = JNI_ERR; DT_RETURN_MARK(GetEnv, jint, (const jint&)ret); - if (vm_created == 0) { + // We can be called by native libraries in the JDK during VM + // initialization, so only bail-out if something seems very wrong. + // Though how would we get here in that case? + if (vm_created == NOT_CREATED) { *penv = nullptr; ret = JNI_EDETACHED; return ret; @@ -3978,7 +3991,8 @@ jint JNICALL jni_GetEnv(JavaVM *vm, void **penv, jint version) { jint JNICALL jni_AttachCurrentThreadAsDaemon(JavaVM *vm, void **penv, void *_args) { HOTSPOT_JNI_ATTACHCURRENTTHREADASDAEMON_ENTRY(vm, penv, _args); - if (vm_created == 0) { + if (vm_created != COMPLETE) { + // Not sure how we could possibly get here. HOTSPOT_JNI_ATTACHCURRENTTHREADASDAEMON_RETURN((uint32_t) JNI_ERR); return JNI_ERR; } diff --git a/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/TestGetCreatedJavaVMs.java b/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/TestGetCreatedJavaVMs.java new file mode 100644 index 00000000000..a84f6d7b327 --- /dev/null +++ b/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/TestGetCreatedJavaVMs.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2023, 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 + * @library /test/lib + * @requires os.family != "Windows" + * @run driver TestGetCreatedJavaVMs + */ +import jdk.test.lib.Utils; +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.process.OutputAnalyzer; + + +public class TestGetCreatedJavaVMs { + public static void main(String args[]) throws Exception { + ProcessBuilder pb = ProcessTools.createNativeTestProcessBuilder("GetCreatedJavaVMs"); + OutputAnalyzer output = ProcessTools.executeProcess(pb); + output.shouldHaveExitValue(0); + output.reportDiagnosticSummary(); + } +} diff --git a/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/exeGetCreatedJavaVMs.c b/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/exeGetCreatedJavaVMs.c new file mode 100644 index 00000000000..a6fe23851f4 --- /dev/null +++ b/test/hotspot/jtreg/runtime/jni/getCreatedJavaVMs/exeGetCreatedJavaVMs.c @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2023, 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. + */ + +/* This code tests concurrent creation of and then attach to a JVM. + * Two threads race to create the JVM, the loser then checks GetCreatedJavaVMs + * and attaches to the returned JVM. Prior to the fix this could crash as the + * JVM is not fully initialized. + */ +#include "jni.h" +#include +#include +#include +#include +#include + +#define NUM_THREADS 2 + +void *thread_runner(void *threadid) { + int tid; + tid = (int)(intptr_t)threadid; + + JavaVM *vm; + JNIEnv *env = 0; + + JavaVMInitArgs vm_args; + JavaVMOption options[0]; + vm_args.version = JNI_VERSION_1_2; + vm_args.nOptions = 0; + vm_args.options = options; + vm_args.ignoreUnrecognized = JNI_FALSE; + + printf("[%d] BEGIN JNI_CreateJavaVM\n", tid); + jint create_res = JNI_CreateJavaVM(&vm, (void **)&env, &vm_args); + printf("[%d] END JNI_CreateJavaVM\n", tid); + + if (create_res != JNI_OK) { + printf("[%d] Error creating JVM: %d\n", tid, create_res); + if (create_res == JNI_EEXIST) { + jsize count; + printf("[%d] BEGIN JNI_GetCreatedJavaVMs\n", tid); + jint get_res = JNI_GetCreatedJavaVMs(&vm, 1, &count); + printf("[%d] END JNI_GetCreatedJavaVMs\n", tid); + + if (get_res != JNI_OK) { + printf("[%d] Error obtaining created VMs: %d\n", tid, get_res); + pthread_exit(NULL); + } else { + printf("[%d] Obtained %d created VMs\n", tid, count); + } + if (count > 0) { + printf("[%d] BEGIN AttachCurrentThread\n", tid); + get_res = (*vm)->AttachCurrentThread(vm, (void **)&env, NULL); + printf("[%d] END AttachCurrentThread - %s\n", tid, + (get_res == JNI_OK ? "succeeded" : "failed")); + if (get_res == JNI_OK) { + (*vm)->DetachCurrentThread(vm); + } + } + pthread_exit(NULL); + } else { + pthread_exit(NULL); + } + } else { + printf("[%d] Created a JVM\n", tid); + } + + pthread_exit(NULL); +} + +int main (int argc, char* argv[]) { + pthread_t threads[NUM_THREADS]; + for (int i = 0; i < NUM_THREADS; i++ ) { + printf("[*] Creating thread %d\n", i); + int status = pthread_create(&threads[i], NULL, thread_runner, (void *)(intptr_t)i); + if (status != 0) { + printf("[*] Error creating thread %d - %d\n", i, status); + exit(-1); + } + } + for (int i = 0; i < NUM_THREADS; i++ ) { + pthread_join(threads[i], NULL); + } + return 0; +}