Bug 46397 - Potential memory leak with TagHandlers and JspIdConsumer
Summary: Potential memory leak with TagHandlers and JspIdConsumer
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Jasper (show other bugs)
Version: unspecified
Hardware: Macintosh Mac OS X 10.4
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-15 00:43 UTC by Jochen Wuttke
Modified: 2008-12-27 13:05 UTC (History)
0 users



Attachments
test case app (12.12 KB, application/octet-stream)
2008-12-15 00:43 UTC, Jochen Wuttke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jochen Wuttke 2008-12-15 00:43:53 UTC
Created attachment 23022 [details]
test case app

As a follow up on the thread on the mailing list, here's my summary again:

Since the JSP Spec says that TagHandlers implementing JspIdConsumer may not be reused, I was expecting them to be collected after the servlet finishes
processing. A few experiments showed that that is not the case.
After delving into the details of the servlet code generated for JSP
pages I discovered that the methods instantiating TagHandlers also
store them in TagHandlerPools using #reuse().

I ran some more experiments to see what  
happens with these TagHandlers. My setup is  
Tomcat 6 + MyFaces + JSF CarDemo, and I'm using AspectJ to trace  
creation and garbage collection of objects implementing JspIdConsumer.  
My experiments indicate the following:

1. JspIdConsumer instances do NOT get garbage collected after a page  
has finished processing.
2. JspIdConsumer instances do NOT always get garbage collected after a  
session times out.
3. SOME JspIdConsumer instances DO get garbage collected when a given  
JSP-generated page is reloaded or accessed in a different session.

To obtain these results I ran Tomcat with very little heap memory (it  
seems it starts fine with the 64M default), loaded the car demo in  
several browser sessions and ran through all the other JSP/Servlet  
examples that come with Tomcat to increase the memory consumption and  
trigger GC. Unfortunately I don't have the time and tools to do a  
proper stress test.

To provide some sort of simpler test case, I attach a WAR to use, but you still need to put the runtime library of AspectJ (aspectjrt.jar) on the classpath when running Tomcat.
The behavior I observe is that whenever I load/reload the page, the  
JspIdConsumer objects get allocated, and some, but never all of
them get GC'd. You should see messages on the console/log like

Adding JspIdConsumer:<someClassName>:<itsHashCode>:<theJspId>

and

Removed <someJspId>

You will see three "Adding messages" for every instance created (it's  
a mess to track this properly),
and you should see one "Removed" message for every GC'd instance.
When I run grep | wc on the output, the "distance" between the number  
of created and GC'd object grows over time and reloads of the test page.
I had this running for a bit, trying to see if after a session timeout  
the remaining instances would be GC'd, but they didn't. I don't know  
what to make of this, but it feels like Tomcat is holding on to some  
of the instances for too long for no good reason.

In any case, considering the specification of JspIdConsumer, which  
states that instances may never be reused, the caching of these  
instances as it is done in current JSPs is unnecessary, and since  
these instances don't seem to be GC'd, this seems to be a memory leak.
Comment 1 Mark Thomas 2008-12-23 08:34:04 UTC
The test case you have provided doesn't do anything. There is no web.xml and no static content. Is there an index.jsp or similar missing?

I should be able to generate a simple test case of my own.
Comment 2 Mark Thomas 2008-12-23 10:32:22 UTC
There is some inefficient use of memory here but no leak. Some tags are incorrectly placed in the pool (I have a patch for that) but all instances are released when the web application is reloaded.

The patch has been applied to trunk and proposed for 6.0.x.
Comment 3 Jochen Wuttke 2008-12-23 10:50:26 UTC
Cool. Thanks for checking.
Comment 4 Mark Thomas 2008-12-27 13:05:00 UTC
The patch has been applied to 6.0.x and will be in 6.0.19 onwards.