From 4bf250fbaaa436591d2a5d6404d022d499750269 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 1 Feb 2021 16:58:02 +0100 Subject: [PATCH] Jetty 9.4.x 5859 classloader leak queuedthreadpool (#5894) * Issue #5859 Fix Classloader leak from QueuedThreadPool Signed-off-by: Jan Bartel --- .../util/thread/PrivilegedThreadFactory.java | 56 +++++++++++++++++++ .../jetty/util/thread/QueuedThreadPool.java | 16 ++++-- .../jetty/util/thread/ShutdownThread.java | 7 ++- .../util/thread/QueuedThreadPoolTest.java | 25 +++++++++ 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/thread/PrivilegedThreadFactory.java diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/PrivilegedThreadFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/PrivilegedThreadFactory.java new file mode 100644 index 00000000000..bc6ad33c892 --- /dev/null +++ b/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 newThread(Supplier newThreadSupplier) + { + return AccessController.doPrivileged(new PrivilegedAction() + { + @Override + public T run() + { + return newThreadSupplier.get(); + } + }); + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index d40b0dbe02f..2408ee877da 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -20,6 +20,8 @@ package org.eclipse.jetty.util.thread; 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 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor @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) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java index ef67b2e4adf..c992d53dcb4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java +++ b/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 @@ import org.eclipse.jetty.util.log.Logger; 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 _lifeCycles = new CopyOnWriteArrayList(); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java index 516e64ea5e1..62bd0fd0e72 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java +++ b/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.Log; 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 class QueuedThreadPoolTest extends AbstractThreadPoolTest 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;