Bug 46221 - Leak WebappClassLoader with commons-logging and log4j
Summary: Leak WebappClassLoader with commons-logging and log4j
Status: RESOLVED WONTFIX
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Unknown (show other bugs)
Version: 5.5.27
Hardware: All All
: P2 enhancement with 5 votes (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-17 04:17 UTC by Arnaud de Bossoreille
Modified: 2011-06-22 18:57 UTC (History)
1 user (show)



Attachments
Simple test case (2.75 KB, application/zip)
2008-11-17 04:20 UTC, Arnaud de Bossoreille
Details
Patch which solves the problem (3.68 KB, patch)
2008-11-17 04:21 UTC, Arnaud de Bossoreille
Details | Diff
Second version of the patch (3.99 KB, patch)
2008-12-03 06:30 UTC, Arnaud de Bossoreille
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arnaud de Bossoreille 2008-11-17 04:17:39 UTC
Tomcat is leaking a few references to the WebappClassLoader instance when the application is stopped and contains commons-logging and log4j packages.

The attached zip file is a very simple test case. I use Yourkit Java Profiler to find the remaining references.

The leak is due to loggers not released by the various containers that are alive until all the webapp is completely unloaded which does happen in my case (start,stop,start,stop...).

The attached patch applies against tomcat 5.5.27 and solves the problem. I am not sure about the its righteousness, especially for the Valve stuff, but I guess it is a good start.
Comment 1 Arnaud de Bossoreille 2008-11-17 04:20:27 UTC
Created attachment 22880 [details]
Simple test case

Use maven 2 to build it, a compiled war cannot be attached because of its size.
Comment 2 Arnaud de Bossoreille 2008-11-17 04:21:32 UTC
Created attachment 22881 [details]
Patch which solves the problem
Comment 3 Peter Rossbach 2008-12-03 03:35:39 UTC
I am not sure that your patch fix the bug really. I have test it with deploy/undeploy via Manager API.
The result is after a while I got a OutOfMemory Exception a perm heap is full.

One thing that I change is deregister containerLog at all Context Valves at StandardPipeline.

StandardPipeline.java
L 273
       while (current != null) {
            if (current instanceof Lifecycle)
                ((Lifecycle) current).stop();
            unregisterValve(current);
            if(current instanceof ValveBase)
                ((ValveBase)current).releaseContainerLog();
        	current = current.getNext();
        }

More hints?

Peter

Comment 4 Arnaud de Bossoreille 2008-12-03 06:27:36 UTC
I am also using the manager via the web interface but with the start/stop links of the webapp.

I am a little bit confused by the plumbing of tomcat's objects, that is why I do not always see what is involved. However it seems that your patch in StandardPipeline.java is better than what I did in StandardContext.java for the same purpose, because it handles more instances than those created by the StandardContext.

My test showed that 3 valves are concerned by the releaseContainerLog calls. But maybe not all of them have a reference on the WebappClassLoader (I did not check). The valves I see (System.err.println(current);) are:

    - org.apache.catalina.core.StandardWrapperValve[jsp]
    - org.apache.catalina.core.StandardWrapperValve[default]
    - org.apache.catalina.core.StandardContextValve[/logging-leak]

I suppose my patch handled only the last one.

Moreover, you may have more valves than I have. This would explain why my patch did not work for you (I guess).

I created the patch I last used so that you can review it (I will add it as a new attachement).
Comment 5 Arnaud de Bossoreille 2008-12-03 06:30:03 UTC
Created attachment 22985 [details]
Second version of the patch
Comment 6 Mark Thomas 2008-12-03 10:44:20 UTC
Using the provided test case I don't see any memory leaks. I do see various references retained after the web application has been stopped but before it is restarted / undeployed.

A leak would be when reloads lead to an increase in the number of instances of a class, typically the webappclassloader, and I don't see that.

Yes, we could do a better job of cleaning up the logger references when the web application stops but that is more an enhancement than a bug.

I'll set up a separate test for multiple deploy/undeploy to see if I can create the issue Peter saw.
Comment 7 Mark Thomas 2008-12-03 14:05:24 UTC
I've done some more testing with several hundred undeploy/deploy cycles and several hundred reload cycles of the test application provided in this report and I do not see any memory leak.

I am changing the status of this issue to enhancement to reflect that clean-up could occur early when a web app is stopped.
Comment 8 Arnaud de Bossoreille 2008-12-04 05:24:28 UTC
Mark, yo're probably right when you say it is more an enhancement than a leak. I could not reproduce it any more. It seems that sometimes the VM is more eager to unload its classes. Some other times it requires dozens of start/stop to finally unload the classes, even with forced GC calls.

However Peter pointed out a real leak which I was not able to reproduce too. Can some objects be initialized in a different order that would explain that kind of behavior?
Comment 9 Mauro Molinari 2009-01-12 03:41:14 UTC
Today I was trying to identify memory leaks in our webapp restart process and I encountered this problem (the logger instance of StandardContext keeps a reference to a logger that is loaded by the webapp class loader, so this can't be garbage collected).
I gave a quick look to the attached patch and it seems to actually solve my problem.
This is clearly a bug (not an enhancement!) because resources are not cleared correctly by org.apache.catalina.core.StandardContext.stop().

I hope this will be fixed soon...

Mauro.
Comment 10 Mauro Molinari 2009-01-12 03:49:00 UTC
After giving a quick look at Tomcat6 sources, this problem seems to affect that version of Tomcat6 too.

Mauro.
Comment 11 Sylvain Laurent 2011-05-21 21:46:36 UTC
I cannot reproduce the problem with tc 7.0.14 nor 6.0.29. Is it really worth fixing this in tc 5 ?
Comment 12 Mark Thomas 2011-06-22 18:51:06 UTC
My original testing was against Tomcat 5.5.x and there was no leak.

Given that the logger may still be used after the web application has stopped (e.g. to log issues during undeployment) I don't - on reflection - see a need for this enhancement.
Comment 13 Mauro Molinari 2011-06-22 18:57:35 UTC
(In reply to comment #12)
> My original testing was against Tomcat 5.5.x and there was no leak.

Well, Tomcat 5.5.x really had the leak! My comment #9 and the attached patch explain and demonstrate this. The fact you don't want to fix it for 5.5 is another story.

Regarding the latest versions of Tomcat 6.x and 7.x, I didn't have the time and the chance to test.