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

Reply via email to