8172726: ForkJoin common pool retains a reference to the thread context class loader
Reviewed-by: martin, psandoz, chegar
--- a/jdk/src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java Mon Feb 06 22:23:46 2017 +0100
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java Tue Feb 07 00:38:22 2017 -0800
@@ -40,6 +40,7 @@
import java.lang.invoke.VarHandle;
import java.security.AccessController;
import java.security.AccessControlContext;
+import java.security.Permission;
import java.security.Permissions;
import java.security.PrivilegedAction;
import java.security.ProtectionDomain;
@@ -132,24 +133,30 @@
* </tr>
* </table>
*
- * <p>The common pool is by default constructed with default
- * parameters, but these may be controlled by setting the following
- * {@linkplain System#getProperty system properties}:
+ * <p>The parameters used to construct the common pool may be controlled by
+ * setting the following {@linkplain System#getProperty system properties}:
* <ul>
* <li>{@code java.util.concurrent.ForkJoinPool.common.parallelism}
* - the parallelism level, a non-negative integer
* <li>{@code java.util.concurrent.ForkJoinPool.common.threadFactory}
- * - the class name of a {@link ForkJoinWorkerThreadFactory}
+ * - the class name of a {@link ForkJoinWorkerThreadFactory}.
+ * The {@linkplain ClassLoader#getSystemClassLoader() system class loader}
+ * is used to load this class.
* <li>{@code java.util.concurrent.ForkJoinPool.common.exceptionHandler}
- * - the class name of a {@link UncaughtExceptionHandler}
+ * - the class name of a {@link UncaughtExceptionHandler}.
+ * The {@linkplain ClassLoader#getSystemClassLoader() system class loader}
+ * is used to load this class.
* <li>{@code java.util.concurrent.ForkJoinPool.common.maximumSpares}
* - the maximum number of allowed extra threads to maintain target
* parallelism (default 256).
* </ul>
- * If a {@link SecurityManager} is present and no factory is
- * specified, then the default pool uses a factory supplying
- * threads that have no {@link Permissions} enabled.
- * The system class loader is used to load these classes.
+ * If no thread factory is supplied via a system property, then the
+ * common pool uses a factory that uses the system class loader as the
+ * {@linkplain Thread#getContextClassLoader() thread context class loader}.
+ * In addition, if a {@link SecurityManager} is present, then
+ * the common pool uses a factory supplying threads that have no
+ * {@link Permissions} enabled.
+ *
* Upon any error in establishing these settings, default parameters
* are used. It is possible to disable or limit the use of threads in
* the common pool by setting the parallelism property to zero, and/or
@@ -638,20 +645,38 @@
*
* @param pool the pool this thread works in
* @return the new worker thread, or {@code null} if the request
- * to create a thread is rejected.
+ * to create a thread is rejected
* @throws NullPointerException if the pool is null
*/
public ForkJoinWorkerThread newThread(ForkJoinPool pool);
}
+ static AccessControlContext contextWithPermissions(Permission ... perms) {
+ Permissions permissions = new Permissions();
+ for (Permission perm : perms)
+ permissions.add(perm);
+ return new AccessControlContext(
+ new ProtectionDomain[] { new ProtectionDomain(null, permissions) });
+ }
+
/**
* Default ForkJoinWorkerThreadFactory implementation; creates a
- * new ForkJoinWorkerThread.
+ * new ForkJoinWorkerThread using the system class loader as the
+ * thread context class loader.
*/
private static final class DefaultForkJoinWorkerThreadFactory
implements ForkJoinWorkerThreadFactory {
+ private static final AccessControlContext ACC = contextWithPermissions(
+ new RuntimePermission("getClassLoader"),
+ new RuntimePermission("setContextClassLoader"));
+
public final ForkJoinWorkerThread newThread(ForkJoinPool pool) {
- return new ForkJoinWorkerThread(pool);
+ return AccessController.doPrivileged(
+ new PrivilegedAction<>() {
+ public ForkJoinWorkerThread run() {
+ return new ForkJoinWorkerThread(
+ pool, ClassLoader.getSystemClassLoader()); }},
+ ACC);
}
}
@@ -3244,26 +3269,20 @@
* An ACC to restrict permissions for the factory itself.
* The constructed workers have no permissions set.
*/
- private static final AccessControlContext INNOCUOUS_ACC;
- static {
- Permissions innocuousPerms = new Permissions();
- innocuousPerms.add(modifyThreadPermission);
- innocuousPerms.add(new RuntimePermission(
- "enableContextClassLoaderOverride"));
- innocuousPerms.add(new RuntimePermission(
- "modifyThreadGroup"));
- INNOCUOUS_ACC = new AccessControlContext(new ProtectionDomain[] {
- new ProtectionDomain(null, innocuousPerms)
- });
- }
+ private static final AccessControlContext ACC = contextWithPermissions(
+ modifyThreadPermission,
+ new RuntimePermission("enableContextClassLoaderOverride"),
+ new RuntimePermission("modifyThreadGroup"),
+ new RuntimePermission("getClassLoader"),
+ new RuntimePermission("setContextClassLoader"));
public final ForkJoinWorkerThread newThread(ForkJoinPool pool) {
- return AccessController.doPrivileged(new PrivilegedAction<>() {
- public ForkJoinWorkerThread run() {
- return new ForkJoinWorkerThread.
- InnocuousForkJoinWorkerThread(pool);
- }}, INNOCUOUS_ACC);
+ return AccessController.doPrivileged(
+ new PrivilegedAction<>() {
+ public ForkJoinWorkerThread run() {
+ return new ForkJoinWorkerThread.
+ InnocuousForkJoinWorkerThread(pool); }},
+ ACC);
}
}
-
}
--- a/jdk/src/java.base/share/classes/java/util/concurrent/ForkJoinWorkerThread.java Mon Feb 06 22:23:46 2017 +0100
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/ForkJoinWorkerThread.java Tue Feb 07 00:38:22 2017 -0800
@@ -36,6 +36,8 @@
package java.util.concurrent;
import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.security.ProtectionDomain;
/**
@@ -88,11 +90,26 @@
}
/**
+ * Version for use by the default pool. Supports setting the
+ * context class loader. This is a separate constructor to avoid
+ * affecting the protected constructor.
+ */
+ ForkJoinWorkerThread(ForkJoinPool pool, ClassLoader ccl) {
+ super("aForkJoinWorkerThread");
+ super.setContextClassLoader(ccl);
+ this.pool = pool;
+ this.workQueue = pool.registerWorker(this);
+ }
+
+ /**
* Version for InnocuousForkJoinWorkerThread.
*/
- ForkJoinWorkerThread(ForkJoinPool pool, ThreadGroup threadGroup,
+ ForkJoinWorkerThread(ForkJoinPool pool,
+ ClassLoader ccl,
+ ThreadGroup threadGroup,
AccessControlContext acc) {
super(threadGroup, null, "aForkJoinWorkerThread");
+ super.setContextClassLoader(ccl);
ThreadLocalRandom.setInheritedAccessControlContext(this, acc);
ThreadLocalRandom.eraseThreadLocals(this); // clear before registering
this.pool = pool;
@@ -179,20 +196,21 @@
/**
* A worker thread that has no permissions, is not a member of any
- * user-defined ThreadGroup, and erases all ThreadLocals after
+ * user-defined ThreadGroup, uses the system class loader as
+ * thread context class loader, and erases all ThreadLocals after
* running each top-level task.
*/
static final class InnocuousForkJoinWorkerThread extends ForkJoinWorkerThread {
/** The ThreadGroup for all InnocuousForkJoinWorkerThreads */
private static final ThreadGroup innocuousThreadGroup =
- java.security.AccessController.doPrivileged(
- new java.security.PrivilegedAction<>() {
- public ThreadGroup run() {
- ThreadGroup group = Thread.currentThread().getThreadGroup();
- for (ThreadGroup p; (p = group.getParent()) != null; )
- group = p;
- return new ThreadGroup(group, "InnocuousForkJoinWorkerThreadGroup");
- }});
+ java.security.AccessController.doPrivileged(
+ new java.security.PrivilegedAction<>() {
+ public ThreadGroup run() {
+ ThreadGroup group = Thread.currentThread().getThreadGroup();
+ for (ThreadGroup p; (p = group.getParent()) != null; )
+ group = p;
+ return new ThreadGroup(group, "InnocuousForkJoinWorkerThreadGroup");
+ }});
/** An AccessControlContext supporting no privileges */
private static final AccessControlContext INNOCUOUS_ACC =
@@ -202,7 +220,10 @@
});
InnocuousForkJoinWorkerThread(ForkJoinPool pool) {
- super(pool, innocuousThreadGroup, INNOCUOUS_ACC);
+ super(pool,
+ ClassLoader.getSystemClassLoader(),
+ innocuousThreadGroup,
+ INNOCUOUS_ACC);
}
@Override // to erase ThreadLocals
@@ -210,11 +231,6 @@
ThreadLocalRandom.eraseThreadLocals(this);
}
- @Override // to always report system loader
- public ClassLoader getContextClassLoader() {
- return ClassLoader.getSystemClassLoader();
- }
-
@Override // to silently fail
public void setUncaughtExceptionHandler(UncaughtExceptionHandler x) { }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/concurrent/tck/ForkJoinPool9Test.java Tue Feb 07 00:38:22 2017 -0800
@@ -0,0 +1,81 @@
+/*
+ * 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 file is available under and governed by the GNU General Public
+ * License version 2 only, as published by the Free Software Foundation.
+ * However, the following notice accompanied the original version of this
+ * file:
+ *
+ * Written by Doug Lea and Martin Buchholz with assistance from
+ * members of JCP JSR-166 Expert Group and released to the public
+ * domain, as explained at
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.util.concurrent.CompletableFuture;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+public class ForkJoinPool9Test extends JSR166TestCase {
+ public static void main(String[] args) {
+ main(suite(), args);
+ }
+
+ public static Test suite() {
+ return new TestSuite(ForkJoinPool9Test.class);
+ }
+
+ /**
+ * Check handling of common pool thread context class loader
+ */
+ public void testCommonPoolThreadContextClassLoader() throws Throwable {
+ if (!testImplementationDetails) return;
+ VarHandle CCL =
+ MethodHandles.privateLookupIn(Thread.class, MethodHandles.lookup())
+ .findVarHandle(Thread.class, "contextClassLoader", ClassLoader.class);
+ ClassLoader systemClassLoader = ClassLoader.getSystemClassLoader();
+ boolean haveSecurityManager = (System.getSecurityManager() != null);
+ CompletableFuture.runAsync(
+ () -> {
+ assertSame(systemClassLoader,
+ Thread.currentThread().getContextClassLoader());
+ assertSame(systemClassLoader,
+ CCL.get(Thread.currentThread()));
+ if (haveSecurityManager)
+ assertThrows(
+ SecurityException.class,
+ () -> System.getProperty("foo"),
+ () -> Thread.currentThread().setContextClassLoader(null));
+
+ // TODO ?
+// if (haveSecurityManager
+// && Thread.currentThread().getClass().getSimpleName()
+// .equals("InnocuousForkJoinWorkerThread"))
+// assertThrows(SecurityException.class, /* ?? */);
+ }).join();
+ }
+
+}
--- a/jdk/test/java/util/concurrent/tck/JSR166TestCase.java Mon Feb 06 22:23:46 2017 +0100
+++ b/jdk/test/java/util/concurrent/tck/JSR166TestCase.java Tue Feb 07 00:38:22 2017 -0800
@@ -46,6 +46,7 @@
* @summary JSR-166 tck tests (whitebox tests allowed)
* @build *
* @modules java.base/java.util.concurrent:open
+ * java.base/java.lang:open
* java.management
* @run junit/othervm/timeout=1000
* -Djsr166.testImplementationDetails=true
@@ -59,6 +60,9 @@
* -Djava.util.concurrent.ForkJoinPool.common.parallelism=1
* -Djava.util.secureRandomSeed=true
* JSR166TestCase
+ * @run junit/othervm/timeout=1000/policy=tck.policy
+ * -Djsr166.testImplementationDetails=true
+ * JSR166TestCase
*/
import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -584,6 +588,7 @@
"AtomicReference9Test",
"AtomicReferenceArray9Test",
"ExecutorCompletionService9Test",
+ "ForkJoinPool9Test",
};
addNamedTestClasses(suite, java9TestClassNames);
}
@@ -594,7 +599,7 @@
/** Returns list of junit-style test method names in given class. */
public static ArrayList<String> testMethodNames(Class<?> testClass) {
Method[] methods = testClass.getDeclaredMethods();
- ArrayList<String> names = new ArrayList<String>(methods.length);
+ ArrayList<String> names = new ArrayList<>(methods.length);
for (Method method : methods) {
if (method.getName().startsWith("test")
&& Modifier.isPublic(method.getModifiers())
@@ -700,7 +705,7 @@
* The first exception encountered if any threadAssertXXX method fails.
*/
private final AtomicReference<Throwable> threadFailure
- = new AtomicReference<Throwable>(null);
+ = new AtomicReference<>(null);
/**
* Records an exception so that it can be rethrown later in the test
@@ -1262,7 +1267,7 @@
}
public void refresh() {}
public String toString() {
- List<Permission> ps = new ArrayList<Permission>();
+ List<Permission> ps = new ArrayList<>();
for (Enumeration<Permission> e = perms.elements(); e.hasMoreElements();)
ps.add(e.nextElement());
return "AdjustablePolicy with permissions " + ps;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/concurrent/tck/tck.policy Tue Feb 07 00:38:22 2017 -0800
@@ -0,0 +1,15 @@
+grant {
+ // Permissions j.u.c. needs directly
+ permission java.lang.RuntimePermission "modifyThread";
+ permission java.lang.RuntimePermission "getClassLoader";
+ permission java.lang.RuntimePermission "setContextClassLoader";
+ permission java.util.PropertyPermission "*", "read";
+ // Permissions needed to change permissions!
+ permission java.security.SecurityPermission "getPolicy";
+ permission java.security.SecurityPermission "setPolicy";
+ permission java.security.SecurityPermission "setSecurityManager";
+ // Permissions needed by the junit test harness
+ permission java.lang.RuntimePermission "accessDeclaredMembers";
+ permission java.io.FilePermission "<<ALL FILES>>", "read";
+ permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
+};