From 8a2ecff4fdc350749a695b5fee2586ee353013bc Mon Sep 17 00:00:00 2001 From: Ivo Studensky Date: Thu, 22 Oct 2015 14:22:41 +0200 Subject: [PATCH] ARTEMIS-274 ActiveMQThreadFactory has to be constructed within doPrivileged block --- .../artemis/utils/ActiveMQThreadFactory.java | 13 +++++-- .../core/client/impl/ServerLocatorImpl.java | 39 ++++++++++++------- .../impl/netty/SharedNioEventLoopGroup.java | 15 +++---- .../io/AbstractSequentialFileFactory.java | 16 +++----- .../core/io/aio/AIOSequentialFileFactory.java | 16 +++----- .../impl/journal/JournalStorageManager.java | 15 +++---- .../remoting/impl/netty/NettyAcceptor.java | 16 +++----- .../server/impl/RemotingServiceImpl.java | 17 ++++---- .../core/server/impl/ActiveMQServerImpl.java | 23 +++++------ 9 files changed, 85 insertions(+), 85 deletions(-) diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadFactory.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadFactory.java index 3c59f34b89..e84e9f4c1e 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadFactory.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadFactory.java @@ -36,6 +36,14 @@ public final class ActiveMQThreadFactory implements ThreadFactory { private final AccessControlContext acc; + /** + * Construct a new instance. The access control context of the calling thread will be the one used to create + * new threads if a security manager is installed. + * + * @param groupName the name of the thread group to assign threads to by default + * @param daemon whether the created threads should be daemon threads + * @param tccl the context class loader of newly created threads + */ public ActiveMQThreadFactory(final String groupName, final boolean daemon, final ClassLoader tccl) { group = new ThreadGroup(groupName + "-" + System.identityHashCode(this)); @@ -45,12 +53,12 @@ public final class ActiveMQThreadFactory implements ThreadFactory { this.daemon = daemon; - this.acc = (System.getSecurityManager() == null) ? null : AccessController.getContext(); + this.acc = AccessController.getContext(); } public Thread newThread(final Runnable command) { // create a thread in a privileged block if running with Security Manager - if (acc != null && System.getSecurityManager() != null) { + if (acc != null) { return AccessController.doPrivileged(new ThreadCreateAction(command), acc); } else { @@ -76,7 +84,6 @@ public final class ActiveMQThreadFactory implements ThreadFactory { t.setDaemon(daemon); t.setPriority(threadPriority); t.setContextClassLoader(tccl); - return t; } diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java index 3f1eead272..73af6fda94 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java @@ -242,7 +242,12 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery private static synchronized ExecutorService getGlobalThreadPool() { if (globalThreadPool == null) { - ThreadFactory factory = new ActiveMQThreadFactory("ActiveMQ-client-global-threads", true, getThisClassLoader()); + ThreadFactory factory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-client-global-threads", true, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); globalThreadPool = Executors.newCachedThreadPool(factory); } @@ -252,11 +257,16 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery private static synchronized ScheduledExecutorService getGlobalScheduledThreadPool() { if (globalScheduledThreadPool == null) { - ThreadFactory factory = new ActiveMQThreadFactory("ActiveMQ-client-global-scheduled-threads", true, getThisClassLoader()); + ThreadFactory factory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-client-global-scheduled-threads", true, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); globalScheduledThreadPool = Executors.newScheduledThreadPool(ActiveMQClient.DEFAULT_SCHEDULED_THREAD_POOL_MAX_SIZE, - factory); + factory); } return globalScheduledThreadPool; @@ -274,7 +284,12 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery else { this.shutdownPool = true; - ThreadFactory factory = new ActiveMQThreadFactory("ActiveMQ-client-factory-threads-" + System.identityHashCode(this), true, getThisClassLoader()); + ThreadFactory factory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-client-factory-threads-" + System.identityHashCode(this), true, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); if (threadPoolMaxSize == -1) { threadPool = Executors.newCachedThreadPool(factory); @@ -283,21 +298,17 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery threadPool = Executors.newFixedThreadPool(threadPoolMaxSize, factory); } - factory = new ActiveMQThreadFactory("ActiveMQ-client-factory-pinger-threads-" + System.identityHashCode(this), true, getThisClassLoader()); + factory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-client-factory-pinger-threads-" + System.identityHashCode(this), true, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); scheduledThreadPool = Executors.newScheduledThreadPool(scheduledThreadPoolMaxSize, factory); } } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return ClientSessionFactoryImpl.class.getClassLoader(); - } - }); - - } - private void instantiateLoadBalancingPolicy() { if (connectionLoadBalancingPolicyClassName == null) { throw new IllegalStateException("Please specify a load balancing policy class name on the session factory"); diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/SharedNioEventLoopGroup.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/SharedNioEventLoopGroup.java index c553c379a9..7656130ccc 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/SharedNioEventLoopGroup.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/SharedNioEventLoopGroup.java @@ -44,14 +44,6 @@ public class SharedNioEventLoopGroup extends NioEventLoopGroup { super(numThreads, factory); } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return ClientSessionFactoryImpl.class.getClassLoader(); - } - }); - } - public static synchronized void forceShutdown() { if (instance != null) { instance.shutdown(); @@ -68,7 +60,12 @@ public class SharedNioEventLoopGroup extends NioEventLoopGroup { } } else { - instance = new SharedNioEventLoopGroup(numThreads, new ActiveMQThreadFactory("ActiveMQ-client-netty-threads", true, getThisClassLoader())); + instance = new SharedNioEventLoopGroup(numThreads, AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-client-netty-threads", true, ClientSessionFactoryImpl.class.getClassLoader()); + } + })); } instance.nioChannelFactoryCount.incrementAndGet(); return instance; diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFileFactory.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFileFactory.java index e27f0c5ffe..16589bf15a 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFileFactory.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/AbstractSequentialFileFactory.java @@ -112,7 +112,12 @@ public abstract class AbstractSequentialFileFactory implements SequentialFileFac } if (isSupportsCallbacks()) { - writeExecutor = Executors.newSingleThreadExecutor(new ActiveMQThreadFactory("ActiveMQ-Asynchronous-Persistent-Writes" + System.identityHashCode(this), true, AbstractSequentialFileFactory.getThisClassLoader())); + writeExecutor = Executors.newSingleThreadExecutor(AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ActiveMQThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-Asynchronous-Persistent-Writes" + System.identityHashCode(this), true, AbstractSequentialFileFactory.class.getClassLoader()); + } + })); } } @@ -177,13 +182,4 @@ public abstract class AbstractSequentialFileFactory implements SequentialFileFac return Arrays.asList(fileNames); } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return AbstractSequentialFileFactory.class.getClassLoader(); - } - }); - - } - } diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/aio/AIOSequentialFileFactory.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/aio/AIOSequentialFileFactory.java index 292d683218..0af2a0ef94 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/aio/AIOSequentialFileFactory.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/aio/AIOSequentialFileFactory.java @@ -186,7 +186,12 @@ public final class AIOSequentialFileFactory extends AbstractSequentialFileFactor this.running.set(true); - pollerExecutor = Executors.newCachedThreadPool(new ActiveMQThreadFactory("ActiveMQ-AIO-poller-pool" + System.identityHashCode(this), true, AIOSequentialFileFactory.getThisClassLoader())); + pollerExecutor = Executors.newCachedThreadPool(AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ActiveMQThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-AIO-poller-pool" + System.identityHashCode(this), true, AIOSequentialFileFactory.class.getClassLoader()); + } + })); pollerExecutor.execute(new PollerRunnable()); } @@ -431,15 +436,6 @@ public final class AIOSequentialFileFactory extends AbstractSequentialFileFactor } } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return AIOSequentialFileFactory.class.getClassLoader(); - } - }); - - } - @Override public String toString() { return AIOSequentialFileFactory.class.getSimpleName() + "(buffersControl.stopped=" + buffersControl.stopped + diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java index cac2c00fea..e0ef75be74 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java @@ -1813,7 +1813,12 @@ public class JournalStorageManager implements StorageManager { cleanupIncompleteFiles(); - singleThreadExecutor = Executors.newSingleThreadExecutor(new ActiveMQThreadFactory("ActiveMQ-IO-SingleThread", true, getThisClassLoader())); + singleThreadExecutor = Executors.newSingleThreadExecutor(AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ActiveMQThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-IO-SingleThread", true, JournalStorageManager.class.getClassLoader()); + } + })); bindingsJournal.start(); @@ -2287,14 +2292,6 @@ public class JournalStorageManager implements StorageManager { } } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return JournalStorageManager.class.getClassLoader(); - } - }); - } - // Inner Classes // ---------------------------------------------------------------------------- diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java index c9bc16aba9..62e93bd331 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java @@ -266,7 +266,12 @@ public class NettyAcceptor implements Acceptor { threadsToUse = this.nioRemotingThreads; } channelClazz = NioServerSocketChannel.class; - eventLoopGroup = new NioEventLoopGroup(threadsToUse, new ActiveMQThreadFactory("activemq-netty-threads", true, getThisClassLoader())); + eventLoopGroup = new NioEventLoopGroup(threadsToUse, AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ActiveMQThreadFactory run() { + return new ActiveMQThreadFactory("activemq-netty-threads", true, ClientSessionFactoryImpl.class.getClassLoader()); + } + })); } bootstrap = new ServerBootstrap(); @@ -682,13 +687,4 @@ public class NettyAcceptor implements Acceptor { cancelled = true; } } - - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return ClientSessionFactoryImpl.class.getClassLoader(); - } - }); - - } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java index 7d3901989f..06428f9d5f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java @@ -188,21 +188,20 @@ public class RemotingServiceImpl implements RemotingService, ConnectionLifeCycle return; } - ClassLoader tccl = AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return Thread.currentThread().getContextClassLoader(); - } - }); - // The remoting service maintains it's own thread pool for handling remoting traffic // If OIO each connection will have it's own thread // If NIO these are capped at nio-remoting-threads which defaults to num cores * 3 // This needs to be a different thread pool to the main thread pool especially for OIO where we may need // to support many hundreds of connections, but the main thread pool must be kept small for better performance - ThreadFactory tFactory = new ActiveMQThreadFactory("ActiveMQ-remoting-threads-" + server.toString() + - "-" + - System.identityHashCode(this), false, tccl); + ThreadFactory tFactory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-remoting-threads-" + server.toString() + + "-" + + System.identityHashCode(this), false, Thread.currentThread().getContextClassLoader()); + } + }); threadPool = Executors.newCachedThreadPool(tFactory); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 7ac30365c3..0cc01723f7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -1405,7 +1405,12 @@ public class ActiveMQServerImpl implements ActiveMQServer { * Executor based on the provided Thread pool. Otherwise we create a new ThreadPool. */ if (serviceRegistry.getExecutorService() == null) { - ThreadFactory tFactory = new ActiveMQThreadFactory("ActiveMQ-server-" + this.toString(), false, getThisClassLoader()); + ThreadFactory tFactory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-server-" + this.toString(), false, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); if (configuration.getThreadPoolMaxSize() == -1) { threadPool = Executors.newCachedThreadPool(tFactory); } @@ -1423,7 +1428,12 @@ public class ActiveMQServerImpl implements ActiveMQServer { * Scheduled ExecutorService otherwise we create a new one. */ if (serviceRegistry.getScheduledExecutorService() == null) { - ThreadFactory tFactory = new ActiveMQThreadFactory("ActiveMQ-scheduled-threads", false, getThisClassLoader()); + ThreadFactory tFactory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-scheduled-threads", false, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); scheduledPool = new ScheduledThreadPoolExecutor(configuration.getScheduledThreadPoolMaxSize(), tFactory); } else { @@ -1784,15 +1794,6 @@ public class ActiveMQServerImpl implements ActiveMQServer { } } - private static ClassLoader getThisClassLoader() { - return AccessController.doPrivileged(new PrivilegedAction() { - public ClassLoader run() { - return ClientSessionFactoryImpl.class.getClassLoader(); - } - }); - - } - /** * Check if journal directory exists or create it (if configured to do so) */