8279488: ProcessBuilder inherits contextClassLoader when spawning a process reaper thread

Reviewed-by: alanb

Author: Roger Riggs

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -24,6 +24,8 @@
  */
 package java.lang;
 
+import jdk.internal.misc.InnocuousThread;
+
 import java.lang.annotation.Native;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -89,22 +91,16 @@ final class ProcessHandleImpl implements ProcessHandle {
                 // of the processReaper threads.
                 ThreadLocalRandom.current();
 
-                ThreadGroup tg = Thread.currentThread().getThreadGroup();
-                while (tg.getParent() != null) tg = tg.getParent();
-                ThreadGroup systemThreadGroup = tg;
-
                 // For a debug build, the stack shadow zone is larger;
                 // Increase the total stack size to avoid potential stack overflow.
                 int debugDelta = "release".equals(System.getProperty("jdk.debug")) ? 0 : (4*4096);
                 final long stackSize = Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize")
                         ? 0 : REAPER_DEFAULT_STACKSIZE + debugDelta;
 
                 ThreadFactory threadFactory = grimReaper -> {
-                    Thread t = new Thread(systemThreadGroup, grimReaper,
-                            "process reaper", stackSize, false);
+                    Thread t = InnocuousThread.newSystemThread("process reaper", grimReaper,
+                            stackSize, Thread.MAX_PRIORITY);
                     t.setDaemon(true);
-                    // A small attempt (probably futile) to avoid priority inversion
-                    t.setPriority(Thread.MAX_PRIORITY);
                     return t;
                 };
 

src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 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
@@ -72,13 +72,15 @@ public static Thread newThread(String name, Runnable target) {
      */
     public static Thread newThread(String name, Runnable target, int priority) {
         if (System.getSecurityManager() == null) {
-            return createThread(name, target, ClassLoader.getSystemClassLoader(), priority);
+            return createThread(name, target, 0L,
+                    ClassLoader.getSystemClassLoader(), priority);
         }
         return AccessController.doPrivileged(
                 new PrivilegedAction<Thread>() {
                     @Override
                     public Thread run() {
-                        return createThread(name, target, ClassLoader.getSystemClassLoader(), priority);
+                        return createThread(name, target, 0L,
+                                ClassLoader.getSystemClassLoader(), priority);
                     }
                 });
     }
@@ -104,28 +106,50 @@ public static Thread newSystemThread(String name, Runnable target) {
      */
     public static Thread newSystemThread(String name, Runnable target, int priority) {
         if (System.getSecurityManager() == null) {
-            return createThread(name, target, null, priority);
+            return createThread(name, target, 0L, null, priority);
         }
         return AccessController.doPrivileged(
                 new PrivilegedAction<Thread>() {
                     @Override
                     public Thread run() {
-                        return createThread(name, target, null, priority);
+                        return createThread(name, target, 0L,
+                                null, priority);
                     }
                 });
     }
 
-    private static Thread createThread(String name, Runnable target, ClassLoader loader, int priority) {
+    /**
+     * Returns a new InnocuousThread with null context class loader.
+     * Thread priority is set to the given priority.
+     */
+    public static Thread newSystemThread(String name, Runnable target,
+                                         long stackSize, int priority) {
+        if (System.getSecurityManager() == null) {
+            return createThread(name, target, stackSize, null, priority);
+        }
+        return AccessController.doPrivileged(
+                new PrivilegedAction<Thread>() {
+                    @Override
+                    public Thread run() {
+                        return createThread(name, target, 0L,
+                                null, priority);
+                    }
+                });
+    }
+
+    private static Thread createThread(String name, Runnable target, long stackSize,
+                                       ClassLoader loader, int priority) {
         Thread t = new InnocuousThread(INNOCUOUSTHREADGROUP,
-                target, name, loader);
+                target, name, stackSize, loader);
         if (priority >= 0) {
             t.setPriority(priority);
         }
         return t;
     }
 
-    private InnocuousThread(ThreadGroup group, Runnable target, String name, ClassLoader tccl) {
-        super(group, target, name, 0L, false);
+    private InnocuousThread(ThreadGroup group, Runnable target, String name,
+                            long stackSize, ClassLoader tccl) {
+        super(group, target, name, stackSize, false);
         UNSAFE.putReferenceRelease(this, INHERITEDACCESSCONTROLCONTEXT, ACC);
         UNSAFE.putReferenceRelease(this, CONTEXTCLASSLOADER, tccl);
     }

test/jdk/java/lang/ProcessBuilder/ProcessReaperCCL.java

@@ -0,0 +1,65 @@
+/*
+ * 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 8269488
+ * @summary verify that Process Reaper threads have a null CCL
+ * @run testng ProcessReaperCCL
+ */
+
+import java.io.File;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class ProcessReaperCCL {
+
+    @Test
+    static void test() throws Exception {
+        // create a class loader
+        File dir = new File(".");
+        URL[] urls = new URL[] {dir.toURI().toURL()};
+        ClassLoader cl = new URLClassLoader(urls);
+        Thread.currentThread().setContextClassLoader(cl);
+
+        // Invoke a subprocess with processBuilder
+        ProcessBuilder pb = new ProcessBuilder(List.of("echo", "abc", "xyz"));
+        Process p = pb.start();
+        CompletableFuture<Process> cf = p.onExit();
+        int exitValue = cf.get().exitValue();
+        Assert.assertEquals(exitValue, 0, "error exit value");
+
+        // Verify all "Process Reaper" threads have a null CCL
+        for (Thread th : Thread.getAllStackTraces().keySet()) {
+            if ("process reaper".equals(th.getName())) {
+                Assert.assertEquals(th.getContextClassLoader(), null, "CCL not null");
+            }
+        }
+    }
+}