Bug 50175 - Enhance memory leak detection by selectively applying methods [PATCH?]
Summary: Enhance memory leak detection by selectively applying methods [PATCH?]
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.2
Hardware: All All
: P2 enhancement with 2 votes (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-28 17:11 UTC by Pid
Modified: 2018-05-15 10:28 UTC (History)
1 user (show)



Attachments
Simple patch illustrating idea (1.72 KB, patch)
2010-10-28 17:12 UTC, Pid
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pid 2010-10-28 17:11:34 UTC
Memory leak detection on application unload can be enhanced by selectively applying the various methods, depending on whether Tomcat is shutting down or undeploying/reloading an application.

Many of the memory leak detection methods can be ignored if a JVM is shutting down.

Non-daemon threads may pose an issue for a system, if they cause the Tomcat process to stay alive and continue to consume memory and/or system resources, so continuing to detect and report these is probably worthwhile.

This could be accomplished by setting a System property during the first phase of the shutdown process and checking for this property during the application undeployment process.
Comment 1 Pid 2010-10-28 17:12:37 UTC
Created attachment 26223 [details]
Simple patch illustrating idea

Adds a system property and checks for it during app undeploy.
Comment 2 Sylvain Laurent 2010-10-31 17:26:22 UTC
What is the final purpose ? speedup the shutdown process or avoid warnings about potential leaks when shutting down ?

Regarding the patch, we also have to take into account the fact that tomcat can be embedded, and I don't know if org.apache.catalina.startup.Catalina.stop() is being called only in non-embedded mode.
Comment 3 Pid 2010-11-01 17:52:38 UTC
(In reply to comment #2)
> What is the final purpose ? speedup the shutdown process or avoid warnings
> about potential leaks when shutting down ?

A bit of both, a User list discussion prompted this.
Some of the leak detection/prevention methods aren't a concern if the JVM is shutting down, but after some thought I'm in two minds about it.
 
> Regarding the patch, we also have to take into account the fact that tomcat can
> be embedded, and I don't know if org.apache.catalina.startup.Catalina.stop() is
> being called only in non-embedded mode.

Fair point. I don't think the patch is complete, I need to think about it more.
Comment 4 Leon Rosenberg 2010-11-02 06:35:02 UTC
I think we should generally distinguish the different usage scenarios. 
For what I know, most people who use tomcat in production environment (in portals or b2c sites) do not use application reloading, because their tomcat-clusters are limited to one, main, application, and the restart of tomcat is safer and faster as the reload. For them most of the detection (if not all) do not make any sense:
- daemon threads will be switched off on shutdown.
- non-daemon threads will prevent shutdown and considered bugs 
- thread-local leakage is not really interested because all threads will be shut down anyway
- what else?

Than there are developers, which maybe use war-reloading (personally i think ctrl-c, arrow-up, enter is faster ;-) ), they will face OOM problems unless they fix all the leaks and/or replace the libs they are using. Still, as a developer I would like to be able to turn it, or have it off by default.

Than there are people who use application reloading in production, and the question remains if this info is useful for the operation of the site. 

However, at least for the first category of people, who never use reloading, this leak detection is pretty useless, isn't it? Therefor, I would support any idea to reduce this 'in-this-usecase-unneeded-check-and-output' :-)
Comment 5 Leon Rosenberg 2011-12-05 15:13:30 UTC
a year passed, any updates on this one? ;-) Can the memory leak detection be switched off completely?
Comment 6 Mark Thomas 2013-10-19 10:06:40 UTC
First of all, the memory leak detection is not going to be disabled. For far too long Tomcat was being blamed for PermGen based OOME errors triggered by web application reloads. The leak detection code has proved invaluable in raising awareness of what the real root causes are and in getting a number of the leaks in popular 3rd party libraries fixed.

I'd disagree with the view that leaks don't matter in production as it depends on how applications are managed. I do agree it is an issue that can safely be ignored some use cases. What would be very useful - both for this issue and for others - would be a way to determine when any Tomcat component is stopped is whether it is being stopped because:
- the JVM is shutting down
- Tomcat is shutting down (e.g. in an embedded scenario)
- just the component is being stopped (e.g. a web app is being undeployed or reloaded)

I've been giving this some thought recently and I have some ideas about how this might be done. I need to turn those ideas into code and see how well they work. If I come up with a viable solution I add it to Tomcat 8.
Comment 7 Rainer Jung 2015-01-05 13:15:30 UTC
I agree it would be nice if we could disable leak detection in case the JVM shuts down. This is a frequent case and in that case the leaks don't matter.

The logs in this case only raise awareness for the sitution when one would switch from recycling the JVM to hot redeployment, embedded mode or similar. OTOH the output often lowers awareness for real problems logged during shutdown. Since many of the leak problems are non-trivial to fix, many users get used to the bunch of messages logged during shutdown and do no longer look at any (other) shutdown messages.

I'm undecided whether the default during JVM shutdown should be doing leak detection or not doing leak detection.

I had a look at how to detect the shutdown but found it hard to do without changing the lifecycle model. But maybe there's a way to detect JVM shutdown outside of our lifecycle model (using a global singleton or similar).
Comment 8 Mark Thomas 2018-05-15 10:28:08 UTC
Implemented in trunk for 9.0.9 onwards.