Jetty 9.4.x 5859 classloader leak queuedthreadpool

jetty-util/src/main/java/org/eclipse/jetty/util/thread/PrivilegedThreadFactory.java

@@ -0,0 +1,56 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.util.thread;
+
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.function.Supplier;
+
+/**
+ * Convenience class to ensure that a new Thread is created
+ * inside a privileged block. 
+ * 
+ * This prevents the Thread constructor
+ * from pinning the caller's context classloader. This happens
+ * when the Thread constructor takes a snapshot of the current
+ * calling context - which contains ProtectionDomains that may
+ * reference the context classloader - and remembers it for the
+ * lifetime of the Thread.
+ */
+class PrivilegedThreadFactory
+{
+    /**
+     * Use a Supplier to make a new thread, calling it within
+     * a privileged block to prevent classloader pinning.
+     * 
+     * @param newThreadSupplier a Supplier to create a fresh thread
+     * @return a new thread, protected from classloader pinning.
+     */
+    static <T extends Thread> T newThread(Supplier<T> newThreadSupplier)
+    {
+        return AccessController.doPrivileged(new PrivilegedAction<T>()
+        {
+            @Override
+            public T run()
+            {
+                return newThreadSupplier.get();
+            }
+        });
+    }
+}

jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java

@@ -20,6 +20,8 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -685,11 +687,15 @@ private boolean addCounts(int deltaThreads, int deltaIdle)
     @Override
     public Thread newThread(Runnable runnable)
     {
-        Thread thread = new Thread(_threadGroup, runnable);
-        thread.setDaemon(isDaemon());
-        thread.setPriority(getThreadsPriority());
-        thread.setName(_name + "-" + thread.getId());
-        return thread;
+        return PrivilegedThreadFactory.newThread(() ->
+        {
+            Thread thread = new Thread(_threadGroup, runnable);
+            thread.setDaemon(isDaemon());
+            thread.setPriority(getThreadsPriority());
+            thread.setName(_name + "-" + thread.getId());
+            thread.setContextClassLoader(getClass().getClassLoader());
+            return thread;
+        });
     }
 
     protected void removeThread(Thread thread)

jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java

@@ -18,6 +18,8 @@
 
 package org.eclipse.jetty.util.thread;
 
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -36,7 +38,10 @@
 public class ShutdownThread extends Thread
 {
     private static final Logger LOG = Log.getLogger(ShutdownThread.class);
-    private static final ShutdownThread _thread = new ShutdownThread();
+    private static final ShutdownThread _thread = PrivilegedThreadFactory.newThread(() ->
+    {
+        return new ShutdownThread();
+    });
 
     private boolean _hooked;
     private final List<LifeCycle> _lifeCycles = new CopyOnWriteArrayList<LifeCycle>();

jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java

@@ -19,6 +19,8 @@
 package org.eclipse.jetty.util.thread;
 
 import java.io.Closeable;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -27,6 +29,7 @@
 import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.log.StacklessLogging;
 import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -833,6 +836,28 @@ public void testDump() throws Exception
         assertThat(count(dump, "QueuedThreadPoolTest.lambda$testDump$"), is(1));
     }
 
+    @Test
+    public void testContextClassLoader() throws Exception
+    {
+        QueuedThreadPool tp = new QueuedThreadPool();
+        try (StacklessLogging stackless = new StacklessLogging(QueuedThreadPool.class))
+        {
+            //change the current thread's classloader to something else
+            Thread.currentThread().setContextClassLoader(new URLClassLoader(new URL[] {}));
+
+            //create a new thread
+            Thread t = tp.newThread(() ->
+            {
+                //the executing thread should be still set to the classloader of the QueuedThreadPool,
+                //not that of the thread that created this thread.
+                assertThat(Thread.currentThread().getContextClassLoader(), Matchers.equalTo(QueuedThreadPool.class.getClassLoader()));
+            });
+
+            //new thread should be set to the classloader of the QueuedThreadPool
+            assertThat(t.getContextClassLoader(), Matchers.equalTo(QueuedThreadPool.class.getClassLoader()));
+        }
+    }
+
     private int count(String s, String p)
     {
         int c = 0;