8344773: SM cleanup in ForkJoinPool

Reviewed-by: alanb

Author: Doug Lea

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java

@@ -37,12 +37,6 @@
 
 import java.lang.Thread.UncaughtExceptionHandler;
 import java.lang.reflect.Field;
-import java.security.AccessController;
-import java.security.AccessControlContext;
-import java.security.Permission;
-import java.security.Permissions;
-import java.security.PrivilegedAction;
-import java.security.ProtectionDomain;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -811,9 +805,7 @@ public class ForkJoinPool extends AbstractExecutorService {
      * initialization.  Since it (or any other created pool) need
      * never be used, we minimize initial construction overhead and
      * footprint to the setup of about a dozen fields, although with
-     * some System property parsing and security processing that takes
-     * far longer than the actual construction when SecurityManagers
-     * are used or properties are set. The common pool is
+     * some System property parsing properties are set. The common pool is
      * distinguished by having a null workerNamePrefix (which is an
      * odd convention, but avoids the need to decode status in factory
      * classes).  It also has PRESET_SIZE config set if parallelism
@@ -839,13 +831,12 @@ public class ForkJoinPool extends AbstractExecutorService {
      *
      * As a more appropriate default in managed environments, unless
      * overridden by system properties, we use workers of subclass
-     * InnocuousForkJoinWorkerThread when there is a SecurityManager
-     * present. These workers have no permissions set, do not belong
-     * to any user-defined ThreadGroup, and clear all ThreadLocals and
-     * reset the ContextClassLoader before (re)activating to execute
-     * top-level task.  The associated mechanics may be JVM-dependent
-     * and must access particular Thread class fields to achieve this
-     * effect.
+     * InnocuousForkJoinWorkerThread for the commonPool.  These
+     * workers do not belong to any user-defined ThreadGroup, and
+     * clear all ThreadLocals and reset the ContextClassLoader before
+     * (re)activating to execute top-level tasks.  The associated
+     * mechanics may be JVM-dependent and must access particular
+     * Thread class fields to achieve this effect.
      *
      * InterruptibleTasks
      * ====================
@@ -917,9 +908,6 @@ public class ForkJoinPool extends AbstractExecutorService {
      * shorts would suffice.  For class WorkQueue, an
      * embedded @Contended region segregates fields most heavily
      * updated by owners from those most commonly read by stealers or
-     * other management.  For class WorkQueue, an embedded padded
-     * region segregates fields (all declared as "int") most heavily
-     * updated by owners from those most commonly read by stealers or
      * other management.
      *
      * Initial sizing and resizing of WorkQueue arrays is an even more
@@ -929,8 +917,10 @@ public class ForkJoinPool extends AbstractExecutorService {
      * direct false-sharing and indirect cases due to GC bookkeeping
      * (cardmarks etc), and reduce the number of resizes, which are
      * not especially fast because they require atomic transfers.
-     * Currently, arrays are initialized to be just large enough to
-     * avoid resizing in most tree-structured tasks. (Maintenance note:
+     * Currently, arrays for workers are initialized to be just large
+     * enough to avoid resizing in most tree-structured tasks, but
+     * larger for external queues where both false-sharing problems
+     * and the need for resizing are more common. (Maintenance note:
      * any changes in fields, queues, or their uses, or JVM layout
      * policies, must be accompanied by re-evaluation of these
      * placement and sizing decisions.)
@@ -1019,6 +1009,12 @@ public class ForkJoinPool extends AbstractExecutorService {
      */
     static final int INITIAL_QUEUE_CAPACITY = 1 << 6;
 
+    /**
+     * Initial capacity of work-stealing queue array for external queues.
+     * Must be a power of two, at least 2. See above.
+     */
+    static final int INITIAL_EXTERNAL_QUEUE_CAPACITY = 1 << 9;
+
     // conversions among short, int, long
     static final int  SMASK           = 0xffff;      // (unsigned) short bits
     static final long LMASK           = 0xffffffffL; // lower 32 bits of long
@@ -1097,21 +1093,6 @@ static long slotOffset(int index) {
         return ((long)index << ASHIFT) + ABASE;
     }
 
-    /**
-     * If there is a security manager, makes sure caller has
-     * permission to modify threads.
-     */
-    @SuppressWarnings("removal")
-    private static void checkPermission() {
-        SecurityManager security; RuntimePermission perm;
-        if ((security = System.getSecurityManager()) != null) {
-            if ((perm = modifyThreadPermission) == null)
-                modifyThreadPermission = perm = // races OK
-                    new RuntimePermission("modifyThread");
-            security.checkPermission(perm);
-        }
-    }
-
     // Nested classes
 
     /**
@@ -1147,64 +1128,9 @@ public static interface ForkJoinWorkerThreadFactory {
     static final class DefaultForkJoinWorkerThreadFactory
         implements ForkJoinWorkerThreadFactory {
         public final ForkJoinWorkerThread newThread(ForkJoinPool pool) {
-            boolean isCommon = (pool.workerNamePrefix == null);
-            @SuppressWarnings("removal")
-            SecurityManager sm = System.getSecurityManager();
-            if (sm != null && isCommon)
-                return newCommonWithACC(pool);
-            else
-                return newRegularWithACC(pool);
-        }
-
-        /*
-         * Create and use static AccessControlContexts only if there
-         * is a SecurityManager. (These can be removed if/when
-         * SecurityManagers are removed from platform.) The ACCs are
-         * immutable and equivalent even when racily initialized, so
-         * they don't require locking, although with the chance of
-         * needlessly duplicate construction.
-         */
-        @SuppressWarnings("removal")
-        static volatile AccessControlContext regularACC, commonACC;
-
-        @SuppressWarnings("removal")
-        static ForkJoinWorkerThread newRegularWithACC(ForkJoinPool pool) {
-            AccessControlContext acc = regularACC;
-            if (acc == null) {
-                Permissions ps = new Permissions();
-                ps.add(new RuntimePermission("getClassLoader"));
-                ps.add(new RuntimePermission("setContextClassLoader"));
-                regularACC = acc =
-                    new AccessControlContext(new ProtectionDomain[] {
-                            new ProtectionDomain(null, ps) });
-            }
-            return AccessController.doPrivileged(
-                new PrivilegedAction<>() {
-                    public ForkJoinWorkerThread run() {
-                        return new ForkJoinWorkerThread(null, pool, true, false);
-                    }}, acc);
-        }
-
-        @SuppressWarnings("removal")
-        static ForkJoinWorkerThread newCommonWithACC(ForkJoinPool pool) {
-            AccessControlContext acc = commonACC;
-            if (acc == null) {
-                Permissions ps = new Permissions();
-                ps.add(new RuntimePermission("getClassLoader"));
-                ps.add(new RuntimePermission("setContextClassLoader"));
-                ps.add(new RuntimePermission("modifyThread"));
-                ps.add(new RuntimePermission("enableContextClassLoaderOverride"));
-                ps.add(new RuntimePermission("modifyThreadGroup"));
-                commonACC = acc =
-                    new AccessControlContext(new ProtectionDomain[] {
-                            new ProtectionDomain(null, ps) });
-            }
-            return AccessController.doPrivileged(
-                new PrivilegedAction<>() {
-                    public ForkJoinWorkerThread run() {
-                        return new ForkJoinWorkerThread.
-                            InnocuousForkJoinWorkerThread(pool);
-                    }}, acc);
+            return ((pool.workerNamePrefix == null) ? // is commonPool
+                    new ForkJoinWorkerThread.InnocuousForkJoinWorkerThread(pool) :
+                    new ForkJoinWorkerThread(null, pool, true, false));
         }
     }
 
@@ -1264,7 +1190,9 @@ final boolean tryLockPhase() {    // seqlock acquire
          */
         WorkQueue(ForkJoinWorkerThread owner, int id, int cfg,
                   boolean clearThreadLocals) {
-            array = new ForkJoinTask<?>[INITIAL_QUEUE_CAPACITY];
+            array = new ForkJoinTask<?>[owner == null ?
+                                        INITIAL_EXTERNAL_QUEUE_CAPACITY :
+                                        INITIAL_QUEUE_CAPACITY];
             this.owner = owner;
             this.config = (clearThreadLocals) ? cfg | CLEAR_TLS : cfg;
         }
@@ -3024,7 +2952,6 @@ public ForkJoinPool(int parallelism,
                         Predicate<? super ForkJoinPool> saturate,
                         long keepAliveTime,
                         TimeUnit unit) {
-        checkPermission();
         int p = parallelism;
         if (p <= 0 || p > MAX_CAP || p > maximumPoolSize || keepAliveTime <= 0L)
             throw new IllegalArgumentException();
@@ -3312,7 +3239,6 @@ public int setParallelism(int size) {
             throw new IllegalArgumentException();
         if ((config & PRESET_SIZE) != 0)
             throw new UnsupportedOperationException("Cannot override System property");
-        checkPermission();
         return getAndSetParallelism(size);
     }
 
@@ -3710,7 +3636,6 @@ public String toString() {
      * may not be rejected.
      */
     public void shutdown() {
-        checkPermission();
         if (workerNamePrefix != null) // not common pool
             tryTerminate(false, true);
     }
@@ -3730,7 +3655,6 @@ public void shutdown() {
      * @return an empty list
      */
     public List<Runnable> shutdownNow() {
-        checkPermission();
         if (workerNamePrefix != null) // not common pool
             tryTerminate(true, true);
         return Collections.emptyList();
@@ -3837,7 +3761,6 @@ public boolean awaitQuiescence(long timeout, TimeUnit unit) {
     @Override
     public void close() {
         if (workerNamePrefix != null) {
-            checkPermission();
             CountDownLatch done = null;
             boolean interrupted = false;
             while ((tryTerminate(interrupted, true) & TERMINATED) == 0) {
@@ -4075,11 +3998,6 @@ public void endCompensatedBlock(ForkJoinPool pool, long post) {
             });
         defaultForkJoinWorkerThreadFactory =
             new DefaultForkJoinWorkerThreadFactory();
-        @SuppressWarnings("removal")
-        ForkJoinPool p = common = (System.getSecurityManager() == null) ?
-            new ForkJoinPool((byte)0) :
-            AccessController.doPrivileged(new PrivilegedAction<>() {
-                    public ForkJoinPool run() {
-                        return new ForkJoinPool((byte)0); }});
+        common = new ForkJoinPool((byte)0);
     }
 }

src/java.base/share/classes/java/util/concurrent/ForkJoinWorkerThread.java

@@ -35,10 +35,6 @@
 
 package java.util.concurrent;
 
-import java.security.AccessController;
-import java.security.AccessControlContext;
-import java.security.PrivilegedAction;
-import java.security.ProtectionDomain;
 import jdk.internal.access.JavaLangAccess;
 import jdk.internal.access.SharedSecrets;
 import jdk.internal.misc.Unsafe;
@@ -84,7 +80,7 @@ public class ForkJoinWorkerThread extends Thread {
         super.setDaemon(true);
         if (handler != null)
             super.setUncaughtExceptionHandler(handler);
-        if (useSystemClassLoader)
+        if (useSystemClassLoader && !clearThreadLocals) // else done by Thread ctor
             super.setContextClassLoader(ClassLoader.getSystemClassLoader());
     }
 
@@ -228,18 +224,23 @@ static boolean hasKnownQueuedWork() {
     }
 
     /**
-     * Clears ThreadLocals, and if necessary resets ContextClassLoader
+     * Clears ThreadLocals
      */
-     final void resetThreadLocals() {
+    final void resetThreadLocals() {
          if (U.getReference(this, THREADLOCALS) != null)
              U.putReference(this, THREADLOCALS, null);
          if (U.getReference(this, INHERITABLETHREADLOCALS) != null)
              U.putReference(this, INHERITABLETHREADLOCALS, null);
-         if ((this instanceof InnocuousForkJoinWorkerThread) &&
-             ((InnocuousForkJoinWorkerThread)this).needCCLReset())
-             super.setContextClassLoader(ClassLoader.getSystemClassLoader());
+         onThreadLocalReset();
      }
 
+    /**
+     * Performs any further cleanup after ThreadLocals are cleared in
+     * method resetThreadLocals
+     */
+    void onThreadLocalReset() {
+    }
+
     private static final Unsafe U = Unsafe.getUnsafe();
     private static final long THREADLOCALS
         = U.objectFieldOffset(Thread.class, "threadLocals");
@@ -248,10 +249,10 @@ final void resetThreadLocals() {
     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
 
     /**
-     * A worker thread that has no permissions, is not a member of any
-     * user-defined ThreadGroup, uses the system class loader as
-     * thread context class loader, and clears all ThreadLocals after
-     * running each top-level task.
+     * A worker thread that is not a member of any user-defined
+     * ThreadGroup, uses the system class loader as thread context
+     * class loader, and clears all ThreadLocals after running each
+     * top-level task.
      */
     static final class InnocuousForkJoinWorkerThread extends ForkJoinWorkerThread {
         /** The ThreadGroup for all InnocuousForkJoinWorkerThreads */
@@ -264,21 +265,20 @@ static final class InnocuousForkJoinWorkerThread extends ForkJoinWorkerThread {
         @Override // to silently fail
         public void setUncaughtExceptionHandler(UncaughtExceptionHandler x) { }
 
-        @Override // paranoically
-        @SuppressWarnings("removal")
+        @Override // to record changes
         public void setContextClassLoader(ClassLoader cl) {
-            if (System.getSecurityManager() != null &&
-                cl != null && ClassLoader.getSystemClassLoader() != cl)
-                throw new SecurityException("setContextClassLoader");
-            resetCCL = true;
-            super.setContextClassLoader(cl);
+            if (ClassLoader.getSystemClassLoader() != cl) {
+                resetCCL = true;
+                super.setContextClassLoader(cl);
+            }
         }
 
-        final boolean needCCLReset() { // get and clear
-            boolean needReset;
-            if (needReset = resetCCL)
+        @Override // to re-establish CCL if necessary
+        final void onThreadLocalReset() {
+            if (resetCCL) {
                 resetCCL = false;
-            return needReset;
+                super.setContextClassLoader(ClassLoader.getSystemClassLoader());
+            }
         }
 
         static ThreadGroup createGroup() {