8287982: Concurrent implicit attach from native threads crashes VM

Reviewed-by: dholmes, rehn

Author: Alan Bateman

make/test/JtregNativeJdk.gmk

@@ -55,7 +55,9 @@ BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeJliLaunchTest := \
 
 # Platform specific setup
 ifeq ($(call isTargetOs, windows), true)
-  BUILD_JDK_JTREG_EXCLUDE += libDirectIO.c libInheritedChannel.c exelauncher.c
+  BUILD_JDK_JTREG_EXCLUDE += libDirectIO.c libInheritedChannel.c \
+      libExplicitAttach.c libImplicitAttach.c \
+      exelauncher.c
 
   WIN_LIB_JAVA := $(SUPPORT_OUTPUTDIR)/native/java.base/libjava/java.lib
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libstringPlatformChars := $(WIN_LIB_JAVA)
@@ -84,6 +86,9 @@ else
   BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libLinkerInvokerUnnamed := $(LIBCXX) -pthread
   BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libLinkerInvokerModule := $(LIBCXX) -pthread
   BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libLoaderLookupInvoker := $(LIBCXX) -pthread
+  BUILD_JDK_JTREG_LIBRARIES_LIBS_libExplicitAttach := -ljvm
+  BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libExplicitAttach := -pthread
+  BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libImplicitAttach := -pthread
   BUILD_JDK_JTREG_EXCLUDE += exerevokeall.c
   ifeq ($(call isTargetOs, linux), true)
     BUILD_JDK_JTREG_LIBRARIES_LIBS_libInheritedChannel := -ljava

src/java.base/share/classes/java/lang/Thread.java

@@ -652,43 +652,48 @@ private static ClassLoader contextClassLoader(Thread parent) {
     @SuppressWarnings("removal")
     Thread(ThreadGroup g, String name, int characteristics, Runnable task,
            long stackSize, AccessControlContext acc) {
-        if (name == null) {
-            throw new InternalError("name cannot be null");
-        }
 
         Thread parent = currentThread();
         boolean attached = (parent == this);   // primordial or JNI attached
-        if (attached && g == null) {
-            throw new InternalError("group cannot be null when attaching");
-        }
 
-        SecurityManager security = System.getSecurityManager();
-        if (g == null) {
-            // the security manager can choose the thread group
-            if (security != null) {
-                g = security.getThreadGroup();
+        if (attached) {
+            if (g == null) {
+                throw new InternalError("group cannot be null when attaching");
             }
-
-            // default to current thread's group
+            this.holder = new FieldHolder(g, task, stackSize, NORM_PRIORITY, false);
+        } else {
+            SecurityManager sm = System.getSecurityManager();
             if (g == null) {
-                g = parent.getThreadGroup();
+                // the security manager can choose the thread group
+                if (sm != null) {
+                    g = sm.getThreadGroup();
+                }
+
+                // default to current thread's group
+                if (g == null) {
+                    g = parent.getThreadGroup();
+                }
             }
-        }
 
-        // permission checks when creating a child Thread
-        if (!attached && security != null) {
-            security.checkAccess(g);
-            if (isCCLOverridden(getClass())) {
-                security.checkPermission(SecurityConstants.SUBCLASS_IMPLEMENTATION_PERMISSION);
+            // permission checks when creating a child Thread
+            if (sm != null) {
+                sm.checkAccess(g);
+                if (isCCLOverridden(getClass())) {
+                    sm.checkPermission(SecurityConstants.SUBCLASS_IMPLEMENTATION_PERMISSION);
+                }
             }
+
+            int priority = Math.min(parent.getPriority(), g.getMaxPriority());
+            this.holder = new FieldHolder(g, task, stackSize, priority, parent.isDaemon());
         }
 
         if (attached && VM.initLevel() < 1) {
             this.tid = 1;  // primordial thread
         } else {
             this.tid = ThreadIdentifiers.next();
         }
-        this.name = name;
+        this.name = (name != null) ? name : genThreadName();
+
         if (acc != null) {
             this.inheritedAccessControlContext = acc;
         } else {
@@ -720,18 +725,6 @@ private static ClassLoader contextClassLoader(Thread parent) {
                 this.contextClassLoader = ClassLoader.getSystemClassLoader();
             }
         }
-
-        int priority;
-        boolean daemon;
-        if (attached) {
-            // primordial or attached thread
-            priority = NORM_PRIORITY;
-            daemon = false;
-        } else {
-            priority = Math.min(parent.getPriority(), g.getMaxPriority());
-            daemon = parent.isDaemon();
-        }
-        this.holder = new FieldHolder(g, task, stackSize, priority, daemon);
     }
 
     /**
@@ -1153,7 +1146,7 @@ private static String checkName(String name) {
      * @see <a href="#inheritance">Inheritance when creating threads</a>
      */
     public Thread() {
-        this(null, genThreadName(), 0, null, 0, null);
+        this(null, null, 0, null, 0, null);
     }
 
     /**
@@ -1174,7 +1167,7 @@ public Thread() {
      * @see <a href="#inheritance">Inheritance when creating threads</a>
      */
     public Thread(Runnable task) {
-        this(null, genThreadName(), 0, task, 0, null);
+        this(null, null, 0, task, 0, null);
     }
 
     /**
@@ -1183,7 +1176,7 @@ public Thread(Runnable task) {
      * This is not a public constructor.
      */
     Thread(Runnable task, @SuppressWarnings("removal") AccessControlContext acc) {
-        this(null, genThreadName(), 0, task, 0, acc);
+        this(null, null, 0, task, 0, acc);
     }
 
     /**
@@ -1216,7 +1209,7 @@ public Thread(Runnable task) {
      * @see <a href="#inheritance">Inheritance when creating threads</a>
      */
     public Thread(ThreadGroup group, Runnable task) {
-        this(group, genThreadName(), 0, task, 0, null);
+        this(group, null, 0, task, 0, null);
     }
 
     /**

test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2022, 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 8287982
+ * @summary Test native threads attaching to the VM with JNI AttachCurrentThread
+ * @requires (os.family == "linux" | os.family == "mac")
+ * @library /test/lib
+ * @compile ExplicitAttach.java
+ * @run main AttachTest ExplicitAttach 1
+ * @run main AttachTest ExplicitAttach 2
+ * @run main AttachTest ExplicitAttach 4
+ */
+
+/**
+ * @test
+ * @summary Test native threads attaching implicitly to the VM by means of an upcall
+ * @requires (os.family == "linux" | os.family == "mac") & (sun.arch.data.model == "64")
+ * @library /test/lib
+ * @compile --enable-preview -source ${jdk.version} ImplicitAttach.java
+ * @run main AttachTest --enable-preview --enable-native-access=ALL-UNNAMED ImplicitAttach 1
+ * @run main AttachTest --enable-preview --enable-native-access=ALL-UNNAMED ImplicitAttach 2
+ * @run main AttachTest --enable-preview --enable-native-access=ALL-UNNAMED ImplicitAttach 4
+ */
+
+import java.util.stream.Stream;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class AttachTest {
+    static final String TEST_CLASSES = System.getProperty("test.classes");
+    static final String JAVA_LIBRARY_PATH = System.getProperty("java.library.path");
+
+    public static void main(String[] args) throws Exception {
+        // prepend -cp ${test.classes} -Djava.library.path=${java.library.path}
+        String[] opts = Stream.concat(Stream.of(
+                        "-cp", TEST_CLASSES,
+                        "-Djava.library.path=" + JAVA_LIBRARY_PATH),
+                        Stream.of(args))
+                .toArray(String[]::new);
+        OutputAnalyzer outputAnalyzer = ProcessTools
+                .executeTestJava(opts)
+                .outputTo(System.out)
+                .errorTo(System.out);
+        int exitValue = outputAnalyzer.getExitValue();
+        if (exitValue != 0)
+            throw new RuntimeException("Test failed");
+    }
+}

test/jdk/java/lang/Thread/jni/AttachCurrentThread/ExplicitAttach.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+import java.util.concurrent.CountDownLatch;
+
+/**
+ * Test native threads attaching to the VM with JNI AttachCurrentThread.
+ */
+public class ExplicitAttach {
+    private static volatile CountDownLatch latch;
+
+    public static void main(String[] args) throws Exception {
+        int threadCount;
+        if (args.length > 0) {
+            threadCount = Integer.parseInt(args[0]);
+        } else {
+            threadCount = 2;
+        }
+        latch = new CountDownLatch(threadCount);
+
+        // start the threads and wait for the threads to call home
+        startThreads(threadCount);
+        latch.await();
+    }
+
+    /**
+     * Invoked by attached threads.
+     */
+    private static void callback() {
+        System.out.println(Thread.currentThread());
+        latch.countDown();
+    }
+
+    /**
+     * Start n native threads that attach to the VM and invoke callback.
+     */
+    private static native void startThreads(int n);
+
+    static {
+        System.loadLibrary("ExplicitAttach");
+    }
+}

test/jdk/java/lang/Thread/jni/AttachCurrentThread/ImplicitAttach.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+import java.lang.foreign.*;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.util.concurrent.CountDownLatch;
+
+/**
+ * Test native threads attaching implicitly to the VM by means of an upcall.
+ */
+public class ImplicitAttach {
+    private static final ValueLayout.OfInt C_INT = ValueLayout.JAVA_INT.withBitAlignment(32);
+    private static final ValueLayout.OfAddress C_POINTER = ValueLayout.ADDRESS.withBitAlignment(64);
+
+    private static volatile CountDownLatch latch;
+
+    public static void main(String[] args) throws Throwable {
+        int threadCount;
+        if (args.length > 0) {
+            threadCount = Integer.parseInt(args[0]);
+        } else {
+            threadCount = 2;
+        }
+        latch = new CountDownLatch(threadCount);
+
+        Linker abi = Linker.nativeLinker();
+
+        // stub to invoke callback
+        MethodHandle callback = MethodHandles.lookup()
+                .findStatic(ImplicitAttach.class, "callback", MethodType.methodType(void.class));
+        MemorySegment upcallStub = abi.upcallStub(callback,
+                FunctionDescriptor.ofVoid(),
+                MemorySession.openImplicit());
+
+        // void start_threads(int count, void *(*f)(void *))
+        SymbolLookup symbolLookup = SymbolLookup.loaderLookup();
+        MemorySegment symbol = symbolLookup.lookup("start_threads").orElseThrow();
+        FunctionDescriptor desc = FunctionDescriptor.ofVoid(C_INT, C_POINTER);
+        MethodHandle start_threads = abi.downcallHandle(symbol, desc);
+
+        // start the threads and wait for the threads to call home
+        start_threads.invoke(threadCount, upcallStub);
+        latch.await();
+    }
+
+    /**
+     * Invoked from native thread.
+     */
+    private static void callback() {
+        System.out.println(Thread.currentThread());
+        latch.countDown();
+    }
+
+    static {
+        System.loadLibrary("ImplicitAttach");
+    }
+}