8279488: ProcessBuilder inherits contextClassLoader when spawning a process reaper thread

Reviewed-by: alanb
This commit is contained in:
Roger Riggs 2022-03-28 15:44:54 +00:00
parent a577656772
commit f0282d7def
3 changed files with 103 additions and 18 deletions

View File

@ -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,10 +91,6 @@ 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);
@ -100,11 +98,9 @@ final class ProcessHandleImpl implements ProcessHandle {
? 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;
};

View File

@ -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 final class InnocuousThread extends Thread {
*/
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 final class InnocuousThread extends Thread {
*/
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);
}

View File

@ -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");
}
}
}
}