Bug 50642 - keepAliveProtection doesn't work
Summary: keepAliveProtection doesn't work
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.30
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-24 10:45 UTC by james
Modified: 2011-04-21 14:10 UTC (History)
0 users



Attachments
ServletContextListener that removes the leaked ClassLoader reference (900 bytes, text/java)
2011-01-24 10:45 UTC, james
Details
Servlet that should trigger this thread to be created (2.64 KB, text/java)
2011-01-27 12:43 UTC, Christopher Schultz
Details
Alternative implementation of the servlet (2.56 KB, text/plain)
2011-01-27 13:22 UTC, Konstantin Kolinko
Details

Note You need to log in before you can comment on or make changes to this bug.
Description james 2011-01-24 10:45:14 UTC
Created attachment 26541 [details]
ServletContextListener that removes the leaked ClassLoader reference

sun.net.www.http.HttpClient.kac.keepAliveTimer.contextClassLoader causes a memory leak as it references the webapp's classloader rather than Tomcat's.

JreMemoryLeakPreventionListener claims to fix this, but only loads sun.net.www.http.HttpClient and does not create the timer thread. Thus the leak still exists.

AFAICT, the attached listener does successfully prevent the leak.
Comment 1 Christopher Schultz 2011-01-24 14:46:24 UTC
Could this be something that is done speculatively?

I'm not sure HttpClient can be rigged to definitely launch it's thread during Tomcat startup, which is where the JreMemoryLeakPreventionListener does most of it's work. There is no URL to which we can guarantee a connection to succeed, unless we fire-up an HTTP connector just for this purpose. That could fail for ay number of reasons and just seems like confusion waiting to happen.

The patch as it stands is somewhat fragile (no error checking, blindly using a Sun-specific class, etc.) but very understandable and could be implemented easily using something similar to the ThreadLocalLeakPreventionListener.
Comment 2 Mark Thomas 2011-01-27 08:41:35 UTC
Looking at the KeepAliveCache more closely, the currently solution is the wrong approach. Because the thread only exists for as long as it is needed and is re-created as necessary it has to be dealt with on web application stop.
Comment 3 Mark Thomas 2011-01-27 11:42:05 UTC
I believe I have a working patch but I have been unable to trigger the creation of the keep-alive thread to test it. A simple test case that triggers this thread creation is needed to resolve this issue.
Comment 4 Christopher Schultz 2011-01-27 12:43:04 UTC
Created attachment 26564 [details]
Servlet that should trigger this thread to be created

It's ugly, but it gets the job done. Stay away from SecurityManager :)

You can run it from CLI, too.
Comment 5 Konstantin Kolinko 2011-01-27 13:22:23 UTC
Created attachment 26566 [details]
Alternative implementation of the servlet

Alternative implementation of the servlet, now using only official API to trigger the issue. Reflection is used only to inspect the state afterwards.

To map the servlet:
    <servlet>
      <servlet-name>issue50642</servlet-name>
      <servlet-class>issue50642.HttpClientServlet</servlet-class>
    </servlet>
    <servlet-mapping>
      <servlet-name>issue50642</servlet-name>
      <url-pattern>/issue50642</url-pattern>
    </servlet-mapping>

When being run as a servlet in TC 7.0.x it prints:

KeepAliveThread: null, cl=null
KeepAliveThread: Thread[Keep-Alive-Timer,8,system], cl=WebappClassLoader
  context: /examples
  delegate: false
  repositories:
    /WEB-INF/classes/
----------> Parent Classloader:
org.apache.catalina.loader.StandardClassLoader@19f953d
Comment 6 Mark Thomas 2011-01-28 08:06:21 UTC
Grr. Eclipse hides system threads by default - hence my failure to re-create this.

Anyway....

Fixed in 7.0.x and will be included in 7.0.7 onwards.

Proposed for 6.0.x
Comment 7 Konstantin Kolinko 2011-02-02 10:46:02 UTC
Fixed in 6.0, will be in 6.0.32.
Comment 8 Dennis Homann 2011-04-21 14:10:05 UTC
The issue is still present in 6.0.32. In more than 50% of my attempts, the KeepAliveThread is not cleared correctly. Debugging and profiling showed that in these cases, the Thread object is still kept around, even though the thread was no longer active. In that case, ThreadGroup#enumerate does not even list it (WebappClassLoader#getThreads), although it's still shown in the profiler (YourKit in my case) and holds a reference to the WebappClassLoader. Even if it was listed by enumerate, WebappClassLoader#clearReferencesThreads would skip it as it is not alive.