From 4a93a000d8a3991f7b9d2b8b9cb663b9903d400d Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 20 Jan 2021 17:13:22 +0100 Subject: [PATCH] Issue #5859 Make safe creation of threads a utility class. Signed-off-by: Jan Bartel --- .../jetty/util/thread/QueuedThreadPool.java | 20 +++---- .../jetty/util/thread/ShutdownThread.java | 13 ++--- .../jetty/util/thread/ThreadCreator.java | 53 +++++++++++++++++++ .../util/thread/QueuedThreadPoolTest.java | 6 --- 4 files changed, 65 insertions(+), 27 deletions(-) create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadCreator.java 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 371f46a91c7e..2f61081e5fac 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 @@ -687,19 +687,15 @@ private boolean addCounts(int deltaThreads, int deltaIdle) @Override public Thread newThread(Runnable runnable) { - return (AccessController.doPrivileged(new PrivilegedAction() + return ThreadCreator.create(() -> { - @Override - public Thread run() - { - Thread thread = new Thread(_threadGroup, runnable); - thread.setDaemon(isDaemon()); - thread.setPriority(getThreadsPriority()); - thread.setName(_name + "-" + thread.getId()); - thread.setContextClassLoader(this.getClass().getClassLoader()); - return thread; - } - })); + Thread thread = new Thread(_threadGroup, runnable); + thread.setDaemon(isDaemon()); + thread.setPriority(getThreadsPriority()); + thread.setName(_name + "-" + thread.getId()); + thread.setContextClassLoader(this.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 27cacaec5727..98f109e9b3ca 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 @@ -38,15 +38,10 @@ public class ShutdownThread extends Thread { private static final Logger LOG = Log.getLogger(ShutdownThread.class); - private static final ShutdownThread _thread = AccessController.doPrivileged(new PrivilegedAction() - { - @Override - public ShutdownThread run() - { - return new ShutdownThread(); - } - - }); + private static final ShutdownThread _thread = ThreadCreator.create(() -> + { + return new ShutdownThread(); + }); private boolean _hooked; private final List _lifeCycles = new CopyOnWriteArrayList(); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadCreator.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadCreator.java new file mode 100644 index 000000000000..407e6de80b60 --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadCreator.java @@ -0,0 +1,53 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 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; + +/** + * ThreadCreator + * + * 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 ThreadCreator +{ + interface Factory + { + T newThread(); + } + + static T create(Factory maker) + { + return AccessController.doPrivileged(new PrivilegedAction() + { + @Override + public T run() + { + return maker.newThread(); + } + }); + } +} 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 f84c512fadb0..6e8a6792d546 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 @@ -840,10 +840,6 @@ public void testDump() throws Exception public void testContextClassLoader() throws Exception { QueuedThreadPool tp = new QueuedThreadPool(); - tp.setMinThreads(1); - tp.setMaxThreads(3); - tp.setIdleTimeout(1000); - tp.setThreadsPriority(Thread.NORM_PRIORITY - 1); try (StacklessLogging stackless = new StacklessLogging(QueuedThreadPool.class)) { //change the current thread's classloader to something else @@ -859,8 +855,6 @@ public void testContextClassLoader() throws Exception //new thread should be set to the classloader of the QueuedThreadPool assertThat(t.getContextClassLoader(), Matchers.equalTo(QueuedThreadPool.class.getClassLoader())); - - t.start(); } }