8344197: SM cleanup in java.util.concurrent

Reviewed-by: alanb, dl

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

@@ -527,20 +527,11 @@ public class ConcurrentSkipListSet<E>
 
     /** Initializes map field; for use in clone. */
     private void setMap(ConcurrentNavigableMap<E,Object> map) {
-        @SuppressWarnings("removal")
-        Field mapField = java.security.AccessController.doPrivileged(
-            (java.security.PrivilegedAction<Field>) () -> {
-                try {
-                    Field f = ConcurrentSkipListSet.class
-                        .getDeclaredField("m");
-                    f.setAccessible(true);
-                    return f;
-                } catch (ReflectiveOperationException e) {
-                    throw new Error(e);
-                }});
         try {
+            Field mapField = ConcurrentSkipListSet.class.getDeclaredField("m");
+            mapField.setAccessible(true);
             mapField.set(this, map);
-        } catch (IllegalAccessException e) {
+        } catch (IllegalAccessException | NoSuchFieldException e) {
             throw new Error(e);
         }
     }

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

@@ -2096,20 +2096,11 @@ public class CopyOnWriteArrayList<E>
 
     /** Initializes the lock; for use when deserializing or cloning. */
     private void resetLock() {
-        @SuppressWarnings("removal")
-        Field lockField = java.security.AccessController.doPrivileged(
-            (java.security.PrivilegedAction<Field>) () -> {
-                try {
-                    Field f = CopyOnWriteArrayList.class
-                        .getDeclaredField("lock");
-                    f.setAccessible(true);
-                    return f;
-                } catch (ReflectiveOperationException e) {
-                    throw new Error(e);
-                }});
         try {
+            Field lockField = CopyOnWriteArrayList.class.getDeclaredField("lock");
+            lockField.setAccessible(true);
             lockField.set(this, new Object());
-        } catch (IllegalAccessException e) {
+        } catch (IllegalAccessException | NoSuchFieldException e) {
             throw new Error(e);
         }
     }

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

@@ -37,16 +37,12 @@ package java.util.concurrent;
 
 import static java.lang.ref.Reference.reachabilityFence;
 import java.lang.ref.Cleaner.Cleanable;
-import java.security.AccessControlContext;
-import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 import jdk.internal.ref.CleanerFactory;
-import sun.security.util.SecurityConstants;
 
 /**
  * Factory and utility methods for {@link Executor}, {@link
@@ -559,27 +555,13 @@ public class Executors {
      */
     private static final class PrivilegedCallable<T> implements Callable<T> {
         final Callable<T> task;
-        @SuppressWarnings("removal")
-        final AccessControlContext acc;
 
-        @SuppressWarnings("removal")
         PrivilegedCallable(Callable<T> task) {
             this.task = task;
-            this.acc = AccessController.getContext();
         }
 
-        @SuppressWarnings("removal")
         public T call() throws Exception {
-            try {
+            return task.call();
-                return AccessController.doPrivileged(
-                    new PrivilegedExceptionAction<T>() {
-                        public T run() throws Exception {
-                            return task.call();
-                        }
-                    }, acc);
-            } catch (PrivilegedActionException e) {
-                throw e.getException();
-            }
         }
 
         public String toString() {
@@ -595,49 +577,26 @@ public class Executors {
             implements Callable<T> {
         final Callable<T> task;
         @SuppressWarnings("removal")
-        final AccessControlContext acc;
         final ClassLoader ccl;
 
         @SuppressWarnings("removal")
         PrivilegedCallableUsingCurrentClassLoader(Callable<T> task) {
-            SecurityManager sm = System.getSecurityManager();
-            if (sm != null) {
-                // Calls to getContextClassLoader from this class
-                // never trigger a security check, but we check
-                // whether our callers have this permission anyways.
-                sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION);
-
-                // Whether setContextClassLoader turns out to be necessary
-                // or not, we fail fast if permission is not available.
-                sm.checkPermission(new RuntimePermission("setContextClassLoader"));
-            }
             this.task = task;
-            this.acc = AccessController.getContext();
             this.ccl = Thread.currentThread().getContextClassLoader();
         }
 
-        @SuppressWarnings("removal")
         public T call() throws Exception {
-            try {
+            Thread t = Thread.currentThread();
-                return AccessController.doPrivileged(
+            ClassLoader cl = t.getContextClassLoader();
-                    new PrivilegedExceptionAction<T>() {
+            if (ccl == cl) {
-                        public T run() throws Exception {
+                return task.call();
-                            Thread t = Thread.currentThread();
+            } else {
-                            ClassLoader cl = t.getContextClassLoader();
+                t.setContextClassLoader(ccl);
-                            if (ccl == cl) {
+                try {
-                                return task.call();
+                    return task.call();
-                            } else {
+                } finally {
-                                t.setContextClassLoader(ccl);
+                    t.setContextClassLoader(cl);
-                                try {
+                }
-                                    return task.call();
-                                } finally {
-                                    t.setContextClassLoader(cl);
-                                }
-                            }
-                        }
-                    }, acc);
-            } catch (PrivilegedActionException e) {
-                throw e.getException();
             }
         }
 
@@ -656,10 +615,7 @@ public class Executors {
         private final String namePrefix;
 
         DefaultThreadFactory() {
-            @SuppressWarnings("removal")
+            group = Thread.currentThread().getThreadGroup();
-            SecurityManager s = System.getSecurityManager();
-            group = (s != null) ? s.getThreadGroup() :
-                                  Thread.currentThread().getThreadGroup();
             namePrefix = "pool-" +
                           poolNumber.getAndIncrement() +
                          "-thread-";
@@ -678,27 +634,14 @@ public class Executors {
     }
 
     /**
-     * Thread factory capturing access control context and class loader.
+     * Thread factory capturing the current class loader.
      */
     private static class PrivilegedThreadFactory extends DefaultThreadFactory {
         @SuppressWarnings("removal")
-        final AccessControlContext acc;
         final ClassLoader ccl;
 
-        @SuppressWarnings("removal")
         PrivilegedThreadFactory() {
             super();
-            SecurityManager sm = System.getSecurityManager();
-            if (sm != null) {
-                // Calls to getContextClassLoader from this class
-                // never trigger a security check, but we check
-                // whether our callers have this permission anyways.
-                sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION);
-
-                // Fail fast
-                sm.checkPermission(new RuntimePermission("setContextClassLoader"));
-            }
-            this.acc = AccessController.getContext();
             this.ccl = Thread.currentThread().getContextClassLoader();
         }
 
@@ -706,13 +649,8 @@ public class Executors {
             return super.newThread(new Runnable() {
                 @SuppressWarnings("removal")
                 public void run() {
-                    AccessController.doPrivileged(new PrivilegedAction<>() {
+                    Thread.currentThread().setContextClassLoader(ccl);
-                        public Void run() {
+                    r.run();
-                            Thread.currentThread().setContextClassLoader(ccl);
-                            r.run();
-                            return null;
-                        }
-                    }, acc);
                 }
             });
         }
@@ -811,9 +749,7 @@ public class Executors {
             super(executor);
             Runnable action = () -> {
                 if (!executor.isShutdown()) {
-                    PrivilegedAction<Void> pa = () -> { executor.shutdown(); return null; };
+                    executor.shutdown();
-                    @SuppressWarnings("removal")
-                    var ignore = AccessController.doPrivileged(pa);
                 }
             };
             cleanable = CleanerFactory.cleaner().register(this, action);

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

@@ -26,8 +26,6 @@ package java.util.concurrent;
 
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Objects;
@@ -688,7 +686,7 @@ public class StructuredTaskScope<T> implements AutoCloseable {
     /**
      * Interrupt all unfinished threads.
      */
-    private void implInterruptAll() {
+    private void interruptAll() {
         flock.threads()
             .filter(t -> t != Thread.currentThread())
             .forEach(t -> {
@@ -698,19 +696,6 @@ public class StructuredTaskScope<T> implements AutoCloseable {
             });
     }
 
-    @SuppressWarnings("removal")
-    private void interruptAll() {
-        if (System.getSecurityManager() == null) {
-            implInterruptAll();
-        } else {
-            PrivilegedAction<Void> pa = () -> {
-                implInterruptAll();
-                return null;
-            };
-            AccessController.doPrivileged(pa);
-        }
-    }
-
     /**
      * Shutdown the task scope if not already shutdown. Return true if this method
      * shutdowns the task scope, false if already shutdown.

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

@@ -39,7 +39,6 @@
 package java.util.concurrent;
 
 import java.io.ObjectStreamField;
-import java.security.AccessControlContext;
 import java.util.Random;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;

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

@@ -26,7 +26,6 @@ package java.util.concurrent;
 
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
-import java.security.Permission;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -48,7 +47,6 @@ import jdk.internal.vm.ThreadContainers;
  */
 class ThreadPerTaskExecutor extends ThreadContainer implements ExecutorService {
     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
-    private static final Permission MODIFY_THREAD = new RuntimePermission("modifyThread");
     private static final VarHandle STATE = MhUtil.findVarHandle(
             MethodHandles.lookup(), "state", int.class);
 
@@ -80,18 +78,6 @@ class ThreadPerTaskExecutor extends ThreadContainer implements ExecutorService {
         return executor;
     }
 
-    /**
-     * Throws SecurityException if there is a security manager set and it denies
-     * RuntimePermission("modifyThread").
-     */
-    @SuppressWarnings("removal")
-    private void checkPermission() {
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(MODIFY_THREAD);
-        }
-    }
-
     /**
      * Throws RejectedExecutionException if the executor has been shutdown.
      */
@@ -143,14 +129,12 @@ class ThreadPerTaskExecutor extends ThreadContainer implements ExecutorService {
 
     @Override
     public void shutdown() {
-        checkPermission();
         if (!isShutdown())
             tryShutdownAndTerminate(false);
     }
 
     @Override
     public List<Runnable> shutdownNow() {
-        checkPermission();
         if (!isTerminated())
             tryShutdownAndTerminate(true);
         return List.of();
@@ -202,7 +186,6 @@ class ThreadPerTaskExecutor extends ThreadContainer implements ExecutorService {
 
     @Override
     public void close() {
-        checkPermission();
         awaitTermination();
     }
 

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

@@ -566,29 +566,6 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
     private static final RejectedExecutionHandler defaultHandler =
         new AbortPolicy();
 
-    /**
-     * Permission required for callers of shutdown and shutdownNow.
-     * We additionally require (see checkShutdownAccess) that callers
-     * have permission to actually interrupt threads in the worker set
-     * (as governed by Thread.interrupt, which relies on
-     * ThreadGroup.checkAccess, which in turn relies on
-     * SecurityManager.checkAccess). Shutdowns are attempted only if
-     * these checks pass.
-     *
-     * All actual invocations of Thread.interrupt (see
-     * interruptIdleWorkers and interruptWorkers) ignore
-     * SecurityExceptions, meaning that the attempted interrupts
-     * silently fail. In the case of shutdown, they should not fail
-     * unless the SecurityManager has inconsistent policies, sometimes
-     * allowing access to a thread and sometimes not. In such cases,
-     * failure to actually interrupt threads may disable or delay full
-     * termination. Other uses of interruptIdleWorkers are advisory,
-     * and failure to actually interrupt will merely delay response to
-     * configuration changes so is not handled exceptionally.
-     */
-    private static final RuntimePermission shutdownPerm =
-        new RuntimePermission("modifyThread");
-
     /**
      * Class Worker mainly maintains interrupt control state for
      * threads running tasks, along with other minor bookkeeping.
@@ -673,10 +650,7 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
         void interruptIfStarted() {
             Thread t;
             if (getState() >= 0 && (t = thread) != null && !t.isInterrupted()) {
-                try {
+                t.interrupt();
-                    t.interrupt();
-                } catch (SecurityException ignore) {
-                }
             }
         }
     }
@@ -749,27 +723,7 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
      */
 
     /**
-     * If there is a security manager, makes sure caller has
+     * Interrupts all threads, even if active.
-     * permission to shut down threads in general (see shutdownPerm).
-     * If this passes, additionally makes sure the caller is allowed
-     * to interrupt each worker thread. This might not be true even if
-     * first check passed, if the SecurityManager treats some threads
-     * specially.
-     */
-    private void checkShutdownAccess() {
-        // assert mainLock.isHeldByCurrentThread();
-        @SuppressWarnings("removal")
-        SecurityManager security = System.getSecurityManager();
-        if (security != null) {
-            security.checkPermission(shutdownPerm);
-            for (Worker w : workers)
-                security.checkAccess(w.thread);
-        }
-    }
-
-    /**
-     * Interrupts all threads, even if active. Ignores SecurityExceptions
-     * (in which case some threads may remain uninterrupted).
      */
     private void interruptWorkers() {
         // assert mainLock.isHeldByCurrentThread();
@@ -780,9 +734,7 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
     /**
      * Interrupts threads that might be waiting for tasks (as
      * indicated by not being locked) so they can check for
-     * termination or configuration changes. Ignores
+     * termination or configuration changes.
-     * SecurityExceptions (in which case some threads may remain
-     * uninterrupted).
      *
      * @param onlyOne If true, interrupt at most one worker. This is
      * called only from tryTerminate when termination is otherwise
@@ -805,7 +757,6 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
                 if (!t.isInterrupted() && w.tryLock()) {
                     try {
                         t.interrupt();
-                    } catch (SecurityException ignore) {
                     } finally {
                         w.unlock();
                     }
@@ -1390,7 +1341,6 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
         final ReentrantLock mainLock = this.mainLock;
         mainLock.lock();
         try {
-            checkShutdownAccess();
             advanceRunState(SHUTDOWN);
             interruptIdleWorkers();
             onShutdown(); // hook for ScheduledThreadPoolExecutor
@@ -1420,7 +1370,6 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
         final ReentrantLock mainLock = this.mainLock;
         mainLock.lock();
         try {
-            checkShutdownAccess();
             advanceRunState(STOP);
             interruptWorkers();
             tasks = drainQueue();

src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java

@@ -37,9 +37,6 @@ package java.util.concurrent.atomic;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.function.IntBinaryOperator;
 import java.util.function.IntUnaryOperator;
 import jdk.internal.misc.Unsafe;
@@ -385,30 +382,16 @@ public abstract class AtomicIntegerFieldUpdater<T> {
         /** class holding the field */
         private final Class<T> tclass;
 
-        @SuppressWarnings("removal")
         AtomicIntegerFieldUpdaterImpl(final Class<T> tclass,
                                       final String fieldName,
                                       final Class<?> caller) {
             final Field field;
             final int modifiers;
             try {
-                field = AccessController.doPrivileged(
+                field = tclass.getDeclaredField(fieldName);
-                    new PrivilegedExceptionAction<Field>() {
-                        public Field run() throws NoSuchFieldException {
-                            return tclass.getDeclaredField(fieldName);
-                        }
-                    });
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                ClassLoader cl = tclass.getClassLoader();
-                ClassLoader ccl = caller.getClassLoader();
-                if ((ccl != null) && (ccl != cl) &&
-                    ((cl == null) || !isAncestor(cl, ccl))) {
-                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
-                }
-            } catch (PrivilegedActionException pae) {
-                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }

src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java

@@ -37,9 +37,6 @@ package java.util.concurrent.atomic;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.function.LongBinaryOperator;
 import java.util.function.LongUnaryOperator;
 import jdk.internal.misc.Unsafe;
@@ -381,29 +378,15 @@ public abstract class AtomicLongFieldUpdater<T> {
         /** class holding the field */
         private final Class<T> tclass;
 
-        @SuppressWarnings("removal")
         CASUpdater(final Class<T> tclass, final String fieldName,
                    final Class<?> caller) {
             final Field field;
             final int modifiers;
             try {
-                field = AccessController.doPrivileged(
+                field = tclass.getDeclaredField(fieldName);
-                    new PrivilegedExceptionAction<Field>() {
-                        public Field run() throws NoSuchFieldException {
-                            return tclass.getDeclaredField(fieldName);
-                        }
-                    });
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                ClassLoader cl = tclass.getClassLoader();
-                ClassLoader ccl = caller.getClassLoader();
-                if ((ccl != null) && (ccl != cl) &&
-                    ((cl == null) || !isAncestor(cl, ccl))) {
-                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
-                }
-            } catch (PrivilegedActionException pae) {
-                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }

src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceArray.java

@@ -329,20 +329,12 @@ public class AtomicReferenceArray<E> implements java.io.Serializable {
             throw new java.io.InvalidObjectException("Not array type");
         if (a.getClass() != Object[].class)
             a = Arrays.copyOf((Object[])a, Array.getLength(a), Object[].class);
-        @SuppressWarnings("removal")
-        Field arrayField = java.security.AccessController.doPrivileged(
-            (java.security.PrivilegedAction<Field>) () -> {
-                try {
-                    Field f = AtomicReferenceArray.class
-                        .getDeclaredField("array");
-                    f.setAccessible(true);
-                    return f;
-                } catch (ReflectiveOperationException e) {
-                    throw new Error(e);
-                }});
         try {
+
+            Field arrayField = AtomicReferenceArray.class.getDeclaredField("array");
+            arrayField.setAccessible(true);
             arrayField.set(this, a);
-        } catch (IllegalAccessException e) {
+        } catch (NoSuchFieldException | IllegalAccessException e) {
             throw new Error(e);
         }
     }

src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java

@@ -37,9 +37,6 @@ package java.util.concurrent.atomic;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.function.BinaryOperator;
 import java.util.function.UnaryOperator;
 import jdk.internal.misc.Unsafe;
@@ -320,7 +317,6 @@ public abstract class AtomicReferenceFieldUpdater<T,V> {
          * screenings fail.
          */
 
-        @SuppressWarnings("removal")
         AtomicReferenceFieldUpdaterImpl(final Class<T> tclass,
                                         final Class<V> vclass,
                                         final String fieldName,
@@ -329,24 +325,11 @@ public abstract class AtomicReferenceFieldUpdater<T,V> {
             final Class<?> fieldClass;
             final int modifiers;
             try {
-                field = AccessController.doPrivileged(
+                field = tclass.getDeclaredField(fieldName);
-                    new PrivilegedExceptionAction<Field>() {
-                        public Field run() throws NoSuchFieldException {
-                            return tclass.getDeclaredField(fieldName);
-                        }
-                    });
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                ClassLoader cl = tclass.getClassLoader();
-                ClassLoader ccl = caller.getClassLoader();
-                if ((ccl != null) && (ccl != cl) &&
-                    ((cl == null) || !isAncestor(cl, ccl))) {
-                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
-                }
                 fieldClass = field.getType();
-            } catch (PrivilegedActionException pae) {
-                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }

src/java.base/share/classes/java/util/concurrent/atomic/Striped64.java

@@ -380,17 +380,12 @@ abstract class Striped64 extends Number {
 
         BASE = MhUtil.findVarHandle(l1, "base", long.class);
         CELLSBUSY = MhUtil.findVarHandle(l1, "cellsBusy", int.class);
-            @SuppressWarnings("removal")
+        try {
-        MethodHandles.Lookup l2 = java.security.AccessController.doPrivileged(
+            MethodHandles.Lookup l2 = MethodHandles.privateLookupIn(Thread.class, l1);
-                    new java.security.PrivilegedAction<>() {
+            THREAD_PROBE = MhUtil.findVarHandle(l2, "threadLocalRandomProbe", int.class);
-                        public MethodHandles.Lookup run() {
+        } catch (ReflectiveOperationException e) {
-                            try {
+            throw new ExceptionInInitializerError(e);
-                                return MethodHandles.privateLookupIn(Thread.class, MethodHandles.lookup());
+        }
-                            } catch (ReflectiveOperationException e) {
-                                throw new ExceptionInInitializerError(e);
-                            }
-                        }});
-        THREAD_PROBE = MhUtil.findVarHandle(l2, "threadLocalRandomProbe", int.class);
     }
 
 }