From: slaurent Date: Thu, 9 Dec 2010 22:11:27 +0000 (+0000) Subject: bug 49159: Improve ThreadLocal memory leak clean-up X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=f7f41d4b80b3a2eb52705d123e05c876f8caafcd;p=tomcat7.0 bug 49159: Improve ThreadLocal memory leak clean-up https://issues.apache.org/bugzilla/show_bug.cgi?id=49159 Use a dedicated thread when calling web application code when it is started and stopped (calls to Listeners, Filters, Servlets). git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1044145 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index 5bc5cd39e..500866191 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -37,6 +37,7 @@ import java.util.Map; import java.util.Set; import java.util.Stack; import java.util.TreeMap; +import java.util.concurrent.Callable; import javax.management.ListenerNotFoundException; import javax.management.MBeanNotificationInfo; @@ -117,6 +118,7 @@ import org.apache.tomcat.JarScanner; import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.modeler.Registry; import org.apache.tomcat.util.scan.StandardJarScanner; +import org.apache.tomcat.util.threads.DedicatedThreadExecutor; /** * Standard implementation of the Context interface. Each @@ -4966,6 +4968,7 @@ public class StandardContext extends ContainerBase } } + DedicatedThreadExecutor temporaryExecutor = new DedicatedThreadExecutor(); try { // Create context attributes that will be required @@ -4992,7 +4995,21 @@ public class StandardContext extends ContainerBase // Configure and call application event listeners if (ok) { - if (!listenerStart()) { + // we do it in a dedicated thread for memory leak protection, in + // case the Listeners registers some ThreadLocals that they + // forget to cleanup + Boolean listenerStarted = + temporaryExecutor.execute(new Callable() { + public Boolean call() throws Exception { + ClassLoader old = bindThread(); + try { + return listenerStart(); + } finally { + unbindThread(old); + } + } + }); + if (!listenerStarted) { log.error( "Error listenerStart"); ok = false; } @@ -5013,20 +5030,48 @@ public class StandardContext extends ContainerBase // Configure and call application filters if (ok) { - if (!filterStart()) { - log.error( "Error filterStart"); + // we do it in a dedicated thread for memory leak protection, in + // case the Filters register some ThreadLocals that they forget + // to cleanup + Boolean filterStarted = + temporaryExecutor.execute(new Callable() { + public Boolean call() throws Exception { + ClassLoader old = bindThread(); + try { + return filterStart(); + } finally { + unbindThread(old); + } + } + }); + if (!filterStarted) { + log.error("Error filterStart"); ok = false; } } // Load and initialize all "load on startup" servlets if (ok) { - loadOnStartup(findChildren()); + // we do it in a dedicated thread for memory leak protection, in + // case the Servlets register some ThreadLocals that they forget + // to cleanup + temporaryExecutor.execute(new Callable() { + public Void call() throws Exception { + ClassLoader old = bindThread(); + try { + loadOnStartup(findChildren()); + return null; + } finally { + unbindThread(old); + } + } + }); } } finally { // Unbinding thread unbindThread(oldCCL); + temporaryExecutor.shutdown(); } // Set available status depending upon startup success @@ -5159,23 +5204,38 @@ public class StandardContext extends ContainerBase try { // Stop our child containers, if any - Container[] children = findChildren(); - for (int i = 0; i < children.length; i++) { - children[i].stop(); - } - - // Stop our filters - filterStop(); - - // Stop ContainerBackgroundProcessor thread - super.threadStop(); - - if ((manager != null) && (manager instanceof Lifecycle)) { - ((Lifecycle) manager).stop(); - } - - // Stop our application listeners - listenerStop(); + final Container[] children = findChildren(); + // we do it in a dedicated thread for memory leak protection, in + // case some webapp code registers some ThreadLocals that they + // forget to cleanup + DedicatedThreadExecutor.executeInOwnThread( + new Callable() { + public Void call() throws Exception { + ClassLoader old = bindThread(); + try { + for (int i = 0; i < children.length; i++) { + children[i].stop(); + } + + // Stop our filters + filterStop(); + + // Stop ContainerBackgroundProcessor thread + threadStop(); + + if ((manager != null) && + (manager instanceof Lifecycle)) { + ((Lifecycle) manager).stop(); + } + + // Stop our application listeners + listenerStop(); + return null; + }finally{ + unbindThread(old); + } + } + }); // Finalize our character set mapper setCharsetMapper(null); diff --git a/java/org/apache/tomcat/util/threads/DedicatedThreadExecutor.java b/java/org/apache/tomcat/util/threads/DedicatedThreadExecutor.java new file mode 100644 index 000000000..fbec1ea43 --- /dev/null +++ b/java/org/apache/tomcat/util/threads/DedicatedThreadExecutor.java @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.threads; + +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; + +/** + * A utility class to execute a {@link Callable} in a dedicated thread. + * It can be used either with an instance to reuse the same thread for each call + * to {@link #execute(Callable)} or with the static method + * {@link #executeInOwnThread(Callable)}. When using an instance, + * {@link #shutdown()} must be called when the instance is no longer needed to + * dispose of the dedicated thread. + */ +public class DedicatedThreadExecutor { + private final SingleThreadFactory singleThreadFactory = + new SingleThreadFactory(); + private final ExecutorService executorService = + Executors.newSingleThreadExecutor(singleThreadFactory); + + /** + * Executes the given {@link Callable} with the thread spawned for the + * current {@link DedicatedThreadExecutor} instance, and returns its result. + * + * @param + * the type of the returned value + * @param callable + * @return + */ + public V execute(final Callable callable) { + final Future futureTask = executorService.submit(callable); + + boolean interrupted = false; + V result; + while (true) { + try { + result = futureTask.get(); + break; + } catch (InterruptedException e) { + // keep waiting + interrupted = true; + } catch (ExecutionException e) { + throw new RuntimeException(e); + } + } + if (interrupted) { + // set interruption status so that the caller may react to it + Thread.currentThread().interrupt(); + } + return result; + } + + /** + * Stops the dedicated thread and waits for its death. + */ + public void shutdown() { + executorService.shutdown(); + if (singleThreadFactory.singleThread != null) { + boolean interrupted = false; + while (true) { + try { + singleThreadFactory.singleThread.join(); + singleThreadFactory.singleThread = null; + break; + } catch (InterruptedException e) { + // keep waiting + interrupted = true; + } + } + if (interrupted) { + // set interruption status so that the caller may react to it + Thread.currentThread().interrupt(); + } + } + } + + /** + * Executes the given {@link Callable} in a new thread and returns the + * result after the thread is stopped. + * + * @param + * @param callable + * @return + */ + public static V executeInOwnThread( + final Callable callable) { + DedicatedThreadExecutor executor = new DedicatedThreadExecutor(); + try { + return executor.execute(callable); + } finally { + executor.shutdown(); + } + + } + + // we use a ThreadFactory so that we can later call Thread.join(). + // Indeed, calling shutdown() on an ExecutorService will eventually stop the + // thread but it might still be alive when execute() returns (race + // condition). + // This can lead to false alarms about potential memory leaks because the + // thread may have a web application class loader for its context class + // loader. + private class SingleThreadFactory implements ThreadFactory { + private volatile Thread singleThread; + + @Override + public Thread newThread(Runnable r) { + if (singleThread != null) { + throw new IllegalStateException( + "should not have been called more than once"); + } + singleThread = new Thread(r); + singleThread.setDaemon(true); + return singleThread; + } + + } +} diff --git a/test/org/apache/tomcat/util/threads/DedicatedThreadExecutorTest.java b/test/org/apache/tomcat/util/threads/DedicatedThreadExecutorTest.java new file mode 100644 index 000000000..22f84311c --- /dev/null +++ b/test/org/apache/tomcat/util/threads/DedicatedThreadExecutorTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.threads; + +import java.util.concurrent.Callable; + +import junit.framework.TestCase; + +public class DedicatedThreadExecutorTest extends TestCase { + private Thread dedicatedThread; + + public void testExecute() { + final Thread testingThread = Thread.currentThread(); + DedicatedThreadExecutor executor = new DedicatedThreadExecutor(); + Long result = executor.execute(new Callable() { + @Override + public Long call() throws Exception { + dedicatedThread = Thread.currentThread(); + DedicatedThreadExecutorTest.assertNotSame(testingThread, + dedicatedThread); + return 123L; + } + }); + assertEquals(123, result.longValue()); + + //check that the same thread is reused + executor.execute(new Callable() { + @Override + public Void call() throws Exception { + DedicatedThreadExecutorTest.assertSame(dedicatedThread, + Thread.currentThread()); + return null; + } + }); + + executor.shutdown(); + assertFalse(dedicatedThread.isAlive()); + } + + public void testExecuteInOwnThread() { + final Thread testingThread = Thread.currentThread(); + Long result = + DedicatedThreadExecutor.executeInOwnThread(new Callable() { + @Override + public Long call() throws Exception { + dedicatedThread = Thread.currentThread(); + DedicatedThreadExecutorTest.assertNotSame(testingThread, + dedicatedThread); + return 456L; + } + }); + assertEquals(456, result.longValue()); + assertFalse(dedicatedThread.isAlive()); + } + +}