Skip to content

Commit

Permalink
Jetty 9.4.x 5859 classloader leak queuedthreadpool (#5894)
Browse files Browse the repository at this point in the history
* Issue #5859 Fix Classloader leak from QueuedThreadPool

Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
janbartel committed Feb 1, 2021
1 parent efbd181 commit 4bf250f
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 6 deletions.
@@ -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();
}
});
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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;
Expand All @@ -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>();
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 4bf250f

Please sign in to comment.