|Summary:||Wrong memory leak detection: executor thread reported as web app thread|
|Product:||Tomcat 6||Reporter:||Marc Guillemot <mguillemot>|
|Component:||Catalina||Assignee:||Tomcat Developers Mailing List <dev>|
|Attachments:||Unit test allowing to generate the wrong log message (but doesn't catpure the log to really test it)|
Description Marc Guillemot 2010-06-09 10:11:08 UTC
Created attachment 25566 [details] Unit test allowing to generate the wrong log message (but doesn't catpure the log to really test it) At Tomcat shutdown I have quite often following message SEVERE: The web application [/] appears to have started a thread named [http-8001-exec-1] but has failed to stop it. This is very likely to create a memory leak. Looking into the details it appears that a request is still being processed. The thread that is detected as leak is not a user thread but Tomcat's executor thread. I'm not sure what would be the right behaviour here: complain about a request that is still being processed or to wait for it. In any case, Tomcat shouldn't complain with current message as it is too misleading and leads to ignoring real memory leak messages. The root cause of the problem is the strategy used to detect threads started by a web app which isn't able to correctly identify threads started by Tomcat itself (contextClassLoader is changed at each request). A solution could be to use a ThreadGroup to identify "Tomcat threads". If you see this as a good solution, I can try to prepare a patch. Same problem occurs in Tomcat 6.
Comment 1 Mark Thomas 2010-06-13 16:28:45 UTC
Tomcat already waits (I think 20s is the default) for requests to complete before shutting down the webapp. A still running request, particularly if deadlocked, would trigger a memory leak so I think this is something that the leak detection should be reporting. I agree a better message is required to make clear the correct root cause in this scenario. I don't have any strong feelings at this point as to the best way to go. At first glance a ThreadGroup looks reasonable.
Comment 2 Marc Guillemot 2010-06-14 03:20:42 UTC
(In reply to comment #1) > Tomcat already waits (I think 20s is the default) for requests to complete > before shutting down the webapp. ??? If you run the provided unit test, you will see that Tomcat doesn't wait 20 secondes for requests to complete. > A still running request, particularly if deadlocked, would trigger a memory > leak so I think this is something that the leak detection should be reporting. > I agree a better message is required to make clear the correct root cause in > this scenario. I don't have any strong feelings at this point as to the best > way to go. At first glance a ThreadGroup looks reasonable. Thinking at it again, it would surely not be bad to have a "Tomcat ThreadGroup" but it wouldn't solve the problem here: user Threads started from a webapp without a specified ThreadGroup would belong to the Tomcat ThreadGroup too. In this case it wouldn't help to distinguish "Tomcat threads" from "user threads".
Comment 3 Mark Thomas 2010-06-25 03:57:05 UTC
Tomcat does wait for requests to complete, I just got the default wait period wrong by an order of magnitude. It waits 2 seconds, not 20 by default. The unloadDelay attribute of the context controls this. I have fixed this issue by examining the stack trace of threads with issues. I decided to use the presence - or not - of the CoyoteAdator.service() call to determine if this was a request thread. It is unlikely (but not impossible) that even a customised Tomcat would have re-written that code. This has been fixed in trunk and will be included in 7.0.1 onwards. I have proposed the same fix for Tomcat 6.
Comment 4 Mark Thomas 2010-06-29 07:32:51 UTC
Fixed in 6.0.x and will be included in 6.0.28 onwards.