Author: markt
Date: Mon Mar 19 19:28:01 2012
New Revision: 1302610
URL: http://svn.apache.org/viewvc?rev=1302610&view=rev
Log:
Refactor the ThreadLocal leak tests
Modified:
tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=1302610&r1=1302609&r2=1302610&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon Mar
19 19:28:01 2012
@@ -41,6 +41,7 @@ import java.security.Permission;
import java.security.PermissionCollection;
import java.security.Policy;
import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Enumeration;
@@ -1089,18 +1090,26 @@ public class WebappClassLoader
// ---------------------------------------------------- ClassLoader Methods
- /**
- * Add the specified URL to the classloader.
- */
+ /**
+ * Add the specified URL to the classloader.
+ */
@Override
- protected void addURL(URL url) {
- super.addURL(url);
- hasExternalRepositories = true;
- repositoryURLs = null;
- }
+ protected void addURL(URL url) {
+ super.addURL(url);
+ hasExternalRepositories = true;
+ repositoryURLs = null;
+ }
/**
+ * Expose this method for use by the unit tests.
+ */
+ protected final Class<?> doDefineClass(String name, byte[] b, int off, int
len,
+ ProtectionDomain protectionDomain) {
+ return super.defineClass(name, b, off, len, protectionDomain);
+ }
+
+ /**
* Find the specified class in our local repositories, if possible. If
* not found, throw <code>ClassNotFoundException</code>.
*
@@ -2568,6 +2577,16 @@ public class WebappClassLoader
}
cl = cl.getParent();
}
+
+ if (o instanceof Collection<?>) {
+ Iterator<?> iter = ((Collection<?>) o).iterator();
+ while (iter.hasNext()) {
+ Object entry = iter.next();
+ if (loadedByThisOrChild(entry)) {
+ return true;
+ }
+ }
+ }
return false;
}
Modified:
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java?rev=1302610&r1=1302609&r2=1302610&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
Mon Mar 19 19:28:01 2012
@@ -17,17 +17,21 @@
package org.apache.catalina.loader;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.List;
+import java.io.InputStream;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Filter;
+import java.util.logging.LogManager;
+import java.util.logging.LogRecord;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+
+import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.Context;
+import org.apache.catalina.core.JreMemoryLeakPreventionListener;
+import org.apache.catalina.core.StandardHost;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.buf.ByteChunk;
@@ -35,119 +39,188 @@ import org.apache.tomcat.util.buf.ByteCh
public class TestWebappClassLoaderThreadLocalMemoryLeak extends TomcatBaseTest
{
@Test
- public void testThreadLocalLeak() throws Exception {
+ public void testThreadLocalLeak1() throws Exception {
Tomcat tomcat = getTomcatInstance();
+ // Need to make sure we see a leak for the right reasons
+ tomcat.getServer().addLifecycleListener(
+ new JreMemoryLeakPreventionListener());
// Must have a real docBase - just use temp
Context ctx = tomcat.addContext("",
System.getProperty("java.io.tmpdir"));
- Tomcat.addServlet(ctx, "leakServlet", new LeakingServlet());
- ctx.addServletMapping("/leak1", "leakServlet");
+ Tomcat.addServlet(ctx, "leakServlet1",
+ "org.apache.tomcat.unittest.TesterLeakingServlet1");
+ ctx.addServletMapping("/leak1", "leakServlet1");
- Tomcat.addServlet(ctx, "leakServlet2", new LeakingServlet2());
- ctx.addServletMapping("/leak2", "leakServlet2");
tomcat.start();
- // This will trigger the timer & thread creation
- ByteChunk chunk = getUrl("http://localhost:"; + getPort() + "/leak1");
- System.out.print("First Threadlocal test response " +
chunk.toString());
-
- chunk = getUrl("http://localhost:"; + getPort() + "/leak2");
- System.out
- .print("Second Threadlocal test response " + chunk.toString());
+ // Configure logging filter to check leak message appears
+ LogValidationFilter f = new LogValidationFilter(
+ "The web application [] created a ThreadLocal with key of");
+ LogManager.getLogManager().getLogger(
+ "org.apache.catalina.loader.WebappClassLoader").setFilter(f);
+
+ // Need to force loading of all web application classes via the web
+ // application class loader
+ loadClass("TesterCounter",
+ (WebappClassLoader) ctx.getLoader().getClassLoader());
+ loadClass("TesterLeakingServlet1",
+ (WebappClassLoader) ctx.getLoader().getClassLoader());
+
+ // This will trigger the ThreadLocal creation
+ int rc = getUrl("http://localhost:"; + getPort() + "/leak1",
+ new ByteChunk(), null);
+
+ // Make sure request is OK
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
- // Stop the context
+ // Destroy the context
ctx.stop();
+ tomcat.getHost().removeChild(ctx);
+ ctx = null;
- // If the thread still exists, we have a thread/memory leak
- try {
- Thread.sleep(1000);
- } catch (InterruptedException ie) {
- // ignore
- }
+ // Make sure we have a memory leak
+ String[] leaks = ((StandardHost) tomcat.getHost())
+ .findReloadedContextMemoryLeaks();
+ Assert.assertNotNull(leaks);
+ Assert.assertTrue(leaks.length > 0);
+
+ // Make sure the message was logged
+ Assert.assertEquals(1, f.getMessageCount());
}
- class LeakingServlet extends HttpServlet {
- private static final long serialVersionUID = 1L;
+ @Test
+ public void testThreadLocalLeak2() throws Exception {
- private ThreadLocal<MyCounter> myThreadLocal = new
ThreadLocal<MyCounter>();
+ Tomcat tomcat = getTomcatInstance();
+ // Need to make sure we see a leak for the right reasons
+ tomcat.getServer().addLifecycleListener(
+ new JreMemoryLeakPreventionListener());
- @Override
- protected void doGet(HttpServletRequest request,
- HttpServletResponse response) throws ServletException,
- IOException {
-
- MyCounter counter = myThreadLocal.get();
- if (counter == null) {
- counter = new MyCounter();
- myThreadLocal.set(counter);
- }
+ // Must have a real docBase - just use temp
+ Context ctx = tomcat.addContext("",
+ System.getProperty("java.io.tmpdir"));
- response.getWriter().println(
- "The current thread served this servlet "
- + counter.getCount() + " times");
- counter.increment();
- }
+ Tomcat.addServlet(ctx, "leakServlet2",
+ "org.apache.tomcat.unittest.TesterLeakingServlet2");
+ ctx.addServletMapping("/leak2", "leakServlet2");
- @Override
- public void destroy() {
- super.destroy();
- // normally not needed, just to make my point
- myThreadLocal = null;
- }
- }
+ tomcat.start();
- class LeakingServlet2 extends HttpServlet {
+ // Configure logging filter to check leak message appears
+ LogValidationFilter f = new LogValidationFilter(
+ "The web application [] created a ThreadLocal with key of");
+ LogManager.getLogManager().getLogger(
+ "org.apache.catalina.loader.WebappClassLoader").setFilter(f);
+
+ // Need to force loading of all web application classes via the web
+ // application class loader
+ loadClass("TesterCounter",
+ (WebappClassLoader) ctx.getLoader().getClassLoader());
+ loadClass("TesterThreadScopedHolder",
+ (WebappClassLoader) ctx.getLoader().getClassLoader());
+ loadClass("TesterLeakingServlet2",
+ (WebappClassLoader) ctx.getLoader().getClassLoader());
+
+ // This will trigger the ThreadLocal creation
+ int rc = getUrl("http://localhost:"; + getPort() + "/leak2",
+ new ByteChunk(), null);
- private static final long serialVersionUID = 1L;
+ // Make sure request is OK
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
- @Override
- protected void doGet(HttpServletRequest request,
- HttpServletResponse response) throws ServletException,
- IOException {
-
- List<MyCounter> counterList = ThreadScopedHolder.getFromHolder();
- MyCounter counter;
- if (counterList == null) {
- counter = new MyCounter();
- ThreadScopedHolder.saveInHolder(Arrays.asList(counter));
- } else {
- counter = counterList.get(0);
- }
+ // Destroy the context
+ ctx.stop();
+ tomcat.getHost().removeChild(ctx);
+ ctx = null;
- response.getWriter().println(
- "The current thread served this servlet "
- + counter.getCount() + " times");
- counter.increment();
- }
+ // Make sure we have a memory leak
+ String[] leaks = ((StandardHost) tomcat.getHost())
+ .findReloadedContextMemoryLeaks();
+ Assert.assertNotNull(leaks);
+ Assert.assertTrue(leaks.length > 0);
+
+ // Make sure the message was logged
+ Assert.assertEquals(1, f.getMessageCount());
}
- static class ThreadScopedHolder {
- private static final ThreadLocal<List<MyCounter>> threadLocal =
- new ThreadLocal<List<MyCounter>>();
- public static void saveInHolder(List<MyCounter> o) {
- threadLocal.set(o);
+ /**
+ * Utility method to ensure that classes are loaded by the
+ * WebappClassLoader. We can't just create classes since they will be
loaded
+ * by the current class loader rather than the WebappClassLoader. This
would
+ * mean that no leak occurred making the test for a leak rather pointless
+ * So, we load the bytes via the current class loader but define the class
+ * with the WebappClassLoader.
+ *
+ * This method assumes that all classes are in the current package.
+ */
+ private void loadClass(String name, WebappClassLoader cl) throws Exception
{
+
+ InputStream is = cl.getResourceAsStream(
+ "org/apache/tomcat/unittest/" + name + ".class");
+ // We know roughly how big the class will be (~ 1K) so allow 2k as a
+ // starting point
+ byte[] classBytes = new byte[2048];
+ int offset = 0;
+ try {
+ int read = is.read(classBytes, offset, classBytes.length-offset);
+ while (read > -1) {
+ offset += read;
+ if (offset == classBytes.length) {
+ // Buffer full - double size
+ byte[] tmp = new byte[classBytes.length * 2];
+ System.arraycopy(classBytes, 0, tmp, 0, classBytes.length);
+ classBytes = tmp;
+ }
+ read = is.read(classBytes, offset, classBytes.length-offset);
+ }
+ Class<?> lpClass = cl.doDefineClass(
+ "org.apache.tomcat.unittest." + name, classBytes, 0,
+ offset, cl.getClass().getProtectionDomain());
+ // Make sure we can create an instance
+ Object obj = lpClass.newInstance();
+ obj.toString();
+ } finally {
+ if (is != null) {
+ try {
+ is.close();
+ } catch (IOException ioe) {
+ // Ignore
+ }
+ }
}
+ }
- public static List<MyCounter> getFromHolder() {
- return threadLocal.get();
+
+ private class LogValidationFilter implements Filter {
+
+ private String targetMessage;
+ private AtomicInteger messageCount = new AtomicInteger(0);
+
+
+ public LogValidationFilter(String targetMessage) {
+ this.targetMessage = targetMessage;
}
- }
- class MyCounter {
- private int count = 0;
- public void increment() {
- count++;
+ public int getMessageCount() {
+ return messageCount.get();
}
- public int getCount() {
- return count;
+
+ @Override
+ public boolean isLoggable(LogRecord record) {
+ String msg = record.getMessage();
+ if (msg != null && msg.contains(targetMessage)) {
+ messageCount.incrementAndGet();
+ }
+
+ return true;
}
}
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
