views:

58

answers:

0

Under the default security manager, if I create an ExecutorService (ThreadPoolExecutor in this case), I cannot shut it down, shutdown() just calls checkPermission("modifyThread") and thus immediately dies:

import java.util.concurrent.*;

class A {
    public static void main( String[] args) {
        Thread ct = Thread.currentThread();
        System.out.println("current thread: " + ct);
        ct.checkAccess(); // we have access to our own thread...
        ThreadPoolExecutor tpe = new ThreadPoolExecutor(
            1, // one core thread
            1, // doesn't matter because queue is unbounded
            0, TimeUnit.SECONDS, // doesn't matter in this case
            new LinkedBlockingQueue<Runnable>(), /* unbound queue for
                                                  * our single thread */
            new ThreadFactory() {
                public Thread newThread(Runnable r) {
                    // obviously never gets called as we don't add any work
                    System.out.println("making thread");
                    return new Thread(r);
                }
            }
        );
        tpe.shutdown(); // raises security exception
    }
}

Sun JDK:

$ java -Djava.security.manager A current thread: Thread[main,5,main] Exception in thread "main" java.security.AccessControlException: access denied (java.lang.RuntimePermission modifyThread) at java.security.AccessControlContext.checkPermission(AccessControlContext.java:323) at java.security.AccessController.checkPermission(AccessController.java:546) at java.lang.SecurityManager.checkPermission(SecurityManager.java:532) at java.util.concurrent.ThreadPoolExecutor.shutdown(ThreadPoolExecutor.java:1094) at A.main(A.java:22)

OpenJDK:

$ java -Djava.security.manager A current thread: Thread[main,5,main] Exception in thread "main" java.security.AccessControlException: access denied (java.lang.RuntimePermission modifyThread) at java.security.AccessControlContext.checkPermission(AccessControlContext.java:342) at java.security.AccessController.checkPermission(AccessController.java:553) at java.lang.SecurityManager.checkPermission(SecurityManager.java:549) at java.util.concurrent.ThreadPoolExecutor.checkShutdownAccess(ThreadPoolExecutor.java:711) at java.util.concurrent.ThreadPoolExecutor.shutdown(ThreadPoolExecutor.java:1351) at A.main(A.java:22)

Why??????? What are the security implications of creating a thread pool that only you control, and shutting it down? Is this a bug in the implementations, or am I missing something?

Let's see what the spec for ExecutorService.shutdown says...

Initiates an orderly shutdown in which previously submitted tasks are executed, but no new tasks will be accepted. Invocation has no additional effect if already shut down.

Throws: SecurityException - if a security manager exists and shutting down this ExecutorService may manipulate threads that the caller is not permitted to modify because it does not hold RuntimePermission("modifyThread"), or the security manager's checkAccess method denies access.

This... is about as vague as it gets. The spec says nothing about any "system threads" being made during the life-cycle of an ExecutorService and furthermore, it lets you supply your own threads which is proof that there should be no "system threads" involved when you do that. (As done above in my sample source)

It feels like the Java SE implementors saw that it's possible for shutdown to raise SecurityException, so they were just like, "oh okay I'll just add a random security check here for compliance"...

The thing is, reading over OpenJDK source (openjdk-6-src-b20-21_jun_2010), it turns out that the only way any thread is ever created, is by calling your supplied ThreadFactory (which is never called in my testcase since I don't create any work, and I don't call prestartCoreThread or preStartAllCoreThreads). The security check is thus done for no apparent reason in OpenJDK's ThreadPoolExecutor (as is done in sun-jdk-1.6 but I don't have the source):

/**
 * Initiates an orderly shutdown in which previously submitted
 * tasks are executed, but no new tasks will be accepted.
 * Invocation has no additional effect if already shut down.
 *
 * @throws SecurityException {@inheritDoc}
 */
public void shutdown() {
    final ReentrantLock mainLock = this.mainLock;
    mainLock.lock();
    try {
        checkShutdownAccess();
        advanceRunState(SHUTDOWN);
        interruptIdleWorkers();
        onShutdown(); // hook for ScheduledThreadPoolExecutor
    } finally {
        mainLock.unlock();
    }
    tryTerminate();
}

checkShutdownAccess is called before doing anything...

/**
 * If there is a security manager, makes sure caller has
 * 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() {
    SecurityManager security = System.getSecurityManager();
    if (security != null) {
        security.checkPermission(shutdownPerm);
        final ReentrantLock mainLock = this.mainLock;
        mainLock.lock();
        try {
            for (Worker w : workers)
                security.checkAccess(w.thread);
        } finally {
            mainLock.unlock();
        }
    }
}

As you can see, it unconditionally invokes checkPermission(shutdownPerm) on the security manager.... shutdownPerm is defined as... private static final RuntimePermission shutdownPerm = new RuntimePermission("modifyThread");

...which makes absolutely no sense as far as I can tell because modifyThread implies access to system threads, and there are no system threads in play here, in fact, there are no threads at all because I didn't submit any work or prestart, and even if there were threads, they'd be my threads because I passed in a ThreadFactory. The spec doesn't say anything about magically dying, other than that if system threads are involved (they aren't), there could be a SecurityException.

Basically, why can't I just remove the line that checks access to system threads? I see no security implication calling for it. And how has nobody else come across this issue??? I've seen a post on an issue tracker where they "resolved" this issue by changing a call to shutdownNow to shutdown, obviously, that didn't fix it for them.