michaelandrepearce commented on a change in pull request #2517: [ARTEMIS-2171]: ThreadPoolExecutor leak under SM due to lack of privileged block. URL: https://github.com/apache/activemq-artemis/pull/2517#discussion_r250702023
########## File path: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/bridge/impl/JMSBridgeImpl.java ########## @@ -1598,25 +1598,30 @@ private static void copyProperties(final Message msg) throws JMSException { * and 1 for the eventual failureHandler) */ private ExecutorService createExecutor() { - ExecutorService service = Executors.newFixedThreadPool(3, new ThreadFactory() { - - ThreadGroup group = new ThreadGroup("JMSBridgeImpl"); - + ExecutorService service = Executors.newFixedThreadPool(3, AccessController.doPrivileged(new PrivilegedAction<ThreadFactory>() { @Override - public Thread newThread(Runnable r) { - final Thread thr = new Thread(group, r); - if (moduleTccl != null) { - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Object run() { - thr.setContextClassLoader(moduleTccl); - return null; + public ThreadFactory run() { + return new ThreadFactory() { Review comment: I know this inst your change, but it highlights a bigger question, as to why is this not using ActiveMQThreadFactory? As this creates threads with the captured context. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services