Re: ThreadLocals, context listeners and classloader leaks
On 26.01.2012 18:00, Jess Holle wrote: On 1/26/2012 10:38 AM, Mark Thomas wrote: OK. ThreadLocals have no place in a web application. Period. If a programmer insists on using them, then it is their responsibility to clean up the mess they leave behind. Tomcat's memory leak detection and prevention code goes some way to clearing up things like this but it is never going to cover every case. Mark Or put another way, you have a choice: 1. Use ThreadLocals the way you'd have assumed you could, but don't expect to ever restart your web app without leaking tons of memory. 2. Use ThreadLocals, but be sure you religiously clean up after yourself by the time your web app is fully shut down. 3. Don't use ThreadLocals. If you use someone else's library that uses ThreadLocals then you'll probably end up in forced into A. That said, there could and arguably should be another choice: 4. Select a special mode in a servlet engine that shuts down all threads that have ever serviced requests for a given web app when it is shutdown (and code your web app to shutdown any threads it creates, obviously!), e.g. after they complete servicing any request in progress. [It could just replace all request threads with new ones after the requests currently in progress complete.] That's assuming the servlet engine is nice enough to provide such a mode. If it did, however, I believe that would resolve any ThreadLocal issues without one having to avoid using a perfect natural and useful Java language feature. I'd argue all servlet engines should really provide this feature for just this reason. That said, I can live with A. Renewing threads is what was implemented some time ago in Tomcat's ThreadLocal leak prevention: http://tomcat.apache.org/tomcat-7.0-doc/config/listeners.html#ThreadLocal_Leak_Prevention_Listener_-_org.apache.catalina.core.ThreadLocalLeakPreventionListener Regards, Rainer - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
ThreadLocals, context listeners and classloader leaks
Hi, I've got some questions regards the use of ThreadLocals in context listeners: (This is a general question, but I tested this with tomcat 6.0.32 only) 1. It's unspecified in the servlet spec 2.5 if a servlet context listener is allowed to take the use of ThreadLocals during contextInitialized(). Is this (also) allowed in tomcat? If the answer of this question is yes: 2. Tomcat invokes the contextInitialized() method with the main thread. At some later time, the main thread (most probably) spawns some more threads (e.g. the http connector/acceptor threads). Because ThreadLocals are inherited from its parent threads, the thread local value references of each ThreadLocal object are copied to the child threads. With the result that even if the reference to the original ThreadLocal instance is removed (for example during contextDestroyed(), the inherited ThreadLocals are still there and reference to the original values - and will most probably remain in the spawned threads forever. (I am aware that tomcat's leak-prevention system / WebClassLoader.clearThreadLocalMap() may remove them, but I can neither rely on that at customer installations nor consider this as a good style) a) Is the main thread usage for context initializers intended? Is this configurable? b) What's the best way to deal with this situation (especially if the avoidance of ThreadLocals in context initializers is not an option)? c) Wouldn't it be better to create an individual thread for (each) context initializer to avoid these kind of problems? Thanks you very much in advance. - Patric - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: ThreadLocals, context listeners and classloader leaks
From: Patric Rufflar [mailto:pat...@rufflar.com] Subject: ThreadLocals, context listeners and classloader leaks It's unspecified in the servlet spec 2.5 if a servlet context listener is allowed to take the use of ThreadLocals during contextInitialized(). I have no idea what the phrase take the use of means; what are you trying to say? Any thread can make use of ThreadLocal, but in a thread pool environment, it's usually silly to do so, unless you're very, very careful. Is this (also) allowed in tomcat? Tomcat makes little effort to prevent stupid programmer tricks. Because ThreadLocals are inherited from its parent threads ??? A ThreadLocal is _not_ inherited from the parent thread, although the _value_ of each InheritableThreadLocal is copied to a spawned thread. the inherited ThreadLocals are still there and reference to the original values - and will most probably remain in the spawned threads forever Which is an example of why you must be very, very careful in your use of ThreadLocal in a pooled environment. Is the main thread usage for context initializers intended? Yes. In Tomcat 7, you can have parallel initialization, but not in Tomcat 6. Read the docs for Engine and Host. What's the best way to deal with this situation (especially if the avoidance of ThreadLocals in context initializers is not an option)? Why is it not an option? Even if your code calls some 3rd-party package to do its work, you can clean up the mess created before returning from the initializer. Wouldn't it be better to create an individual thread for (each) context initializer to avoid these kind of problems? Tomcat can't work around every possible programming silliness. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
RE: ThreadLocals, context listeners and classloader leaks
I have no idea what the phrase take the use of means; what are you trying to say? I'd like to know if there's some statement from the tomcat team if the usage of ThreadLocals within contextInitialized() is discouraged or even not supported. ??? A ThreadLocal is _not_ inherited from the parent thread, although the _value_ of each InheritableThreadLocal is copied to a spawned thread. I meant exactly that. Consider the situation: 1. contextInitializer() sets value A to the ThreadLocal X in thread main 2. childs threads get spawned from main thread, now we have more than one ThreadLocal which references to value A 3. Reference to ThreadLocal X gets dropped, but references to value A still exist - without being able to remove them. Why is it not an option? Even if your code calls some 3rd-party package to do its work, you can clean up the mess created before returning from the initializer. This means that everyone which uses tomcat has to deeply analyze which ThreadLocals are created under which circumstances, and everytime a 3rd party library is added/upgraded/replaced he has to do this again. Beside the risk, that something will be missed... Tomcat can't work around every possible programming silliness. Agreed. But wouldn't it be much easier for everyone if tomcat would always use a separate thread for each context initializer (even if parallel initialization is disabled) than to rely on that every possible programming silliness in 3rd-party code will be fixed? (BTW, Log4j is one of those buggy libraries - see https://issues.apache.org/bugzilla/show_bug.cgi?id=50486 for details) - Patric Am 26.01.2012 15:50, schrieb Caldarale, Charles R: From: Patric Rufflar [mailto:pat...@rufflar.com] Subject: ThreadLocals, context listeners and classloader leaks It's unspecified in the servlet spec 2.5 if a servlet context listener is allowed to take the use of ThreadLocals during contextInitialized(). I have no idea what the phrase take the use of means; what are you trying to say? Any thread can make use of ThreadLocal, but in a thread pool environment, it's usually silly to do so, unless you're very, very careful. Is this (also) allowed in tomcat? Tomcat makes little effort to prevent stupid programmer tricks. Because ThreadLocals are inherited from its parent threads ??? A ThreadLocal is _not_ inherited from the parent thread, although the _value_ of each InheritableThreadLocal is copied to a spawned thread. the inherited ThreadLocals are still there and reference to the original values - and will most probably remain in the spawned threads forever Which is an example of why you must be very, very careful in your use of ThreadLocal in a pooled environment. Is the main thread usage for context initializers intended? Yes. In Tomcat 7, you can have parallel initialization, but not in Tomcat 6. Read the docs for Engine and Host. What's the best way to deal with this situation (especially if the avoidance of ThreadLocals in context initializers is not an option)? Why is it not an option? Even if your code calls some 3rd-party package to do its work, you can clean up the mess created before returning from the initializer. Wouldn't it be better to create an individual thread for (each) context initializer to avoid these kind of problems? Tomcat can't work around every possible programming silliness. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers. - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: ThreadLocals, context listeners and classloader leaks
From: Patric Rufflar [mailto:pat...@rufflar.com] Subject: RE: ThreadLocals, context listeners and classloader leaks 1. contextInitializer() sets value A to the ThreadLocal X in thread main 2. childs threads get spawned from main thread, now we have more than one ThreadLocal which references to value A No; again, a ThreadLocal is _not_ inherited, but an InheritableThreadLocal is. These are different animals. 3. Reference to ThreadLocal X gets dropped, but references to value A still exist - without being able to remove them. Why can't they be removed? (The code to do so is ugly, but readily findable with Google.) But wouldn't it be much easier for everyone if tomcat would always use a separate thread for each context initializer Again, that would fix _one_ stupid programmer trick, out of the uncountable number of potential errors. Note that Tomcat 7 includes a ThreadLocalLeakPreventionListener, but it is only invoked during undeployment of a Context to clean up Executor thread pools. It would certainly be possible to use this logic upon return from an initializer; filing a bugzilla enhancement request would at least allow some discussion by the committers. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
RE: ThreadLocals, context listeners and classloader leaks
Am 26.01.2012 16:59, schrieb Caldarale, Charles R: No; again, a ThreadLocal is _not_ inherited, but an InheritableThreadLocal is. These are different animals. 1. A InheritableThreadLocal is (extends) a ThreadLocal. 2. Surprise: A InheritableThreadLocal is _not_ used for the Thread.inheritableThreadLocals field/mechanism when creating a new thread (at least not in oracle jdk 6, see Thread.class source for details) - for this, a ThreadLocal.class instance is used, too. So the statement we have more than one ThreadLocal which references to value A seems correct to me. Why can't they be removed? (The code to do so is ugly, but readily findable with Google.) I think you mean by traversing all threads and do some reflection actions on them? Please note that I already included feasible workarounds into my projects - so for me, this issue is not a show stopper anymore. But: The intention of my original mail was to highlight the problem with ThreadLocals/Context initializers and to suggest an improvement to tomcat to prevent others from falling into this pit. Secondly, from an economic perspective such an improvement in tomcat would be much cheaper than to include such hacks inside all the projects all over the world. Again, that would fix _one_ stupid programmer trick, out of the uncountable number of potential errors. I am interested in these disadvantages. What kind of potential errors do you see if an individual thread would be used for each context initialization? - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: ThreadLocals, context listeners and classloader leaks
On 26/01/2012 15:16, Patric Rufflar wrote: I have no idea what the phrase take the use of means; what are you trying to say? I'd like to know if there's some statement from the tomcat team if the usage of ThreadLocals within contextInitialized() is discouraged or even not supported. OK. ThreadLocals have no place in a web application. Period. If a programmer insists on using them, then it is their responsibility to clean up the mess they leave behind. Tomcat's memory leak detection and prevention code goes some way to clearing up things like this but it is never going to cover every case. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: ThreadLocals, context listeners and classloader leaks
On 1/26/2012 10:38 AM, Mark Thomas wrote: OK. ThreadLocals have no place in a web application. Period. If a programmer insists on using them, then it is their responsibility to clean up the mess they leave behind. Tomcat's memory leak detection and prevention code goes some way to clearing up things like this but it is never going to cover every case. Mark Or put another way, you have a choice: 1. Use ThreadLocals the way you'd have assumed you could, but don't expect to ever restart your web app without leaking tons of memory. 2. Use ThreadLocals, but be sure you religiously clean up after yourself by the time your web app is fully shut down. 3. Don't use ThreadLocals. If you use someone else's library that uses ThreadLocals then you'll probably end up in forced into A. That said, there could and arguably should be another choice: 4. Select a special mode in a servlet engine that shuts down all threads that have ever serviced requests for a given web app when it is shutdown (and code your web app to shutdown any threads it creates, obviously!), e.g. after they complete servicing any request in progress. [It could just replace all request threads with new ones after the requests currently in progress complete.] That's assuming the servlet engine is nice enough to provide such a mode. If it did, however, I believe that would resolve any ThreadLocal issues without one having to avoid using a perfect natural and useful Java language feature. I'd argue all servlet engines should really provide this feature for just this reason. That said, I can live with A. -- Jess Holle
RE: ThreadLocals, context listeners and classloader leaks
From: Jess Holle [mailto:je...@ptc.com] Subject: Re: ThreadLocals, context listeners and classloader leaks That said, there could and arguably should be another choice: I'll suggest something more radical: define a class such as ScopeLocal where values are added to and removed from threads as they enter and leave a scope boundary. For servlet engine purposes, typical scopes would be webapp, session, request, servlet, etc. Doing it properly (and efficiently) might well require JVM intervention, but it would certainly help in providing a useful mechanism for app writers. Obviously, there's lots of definition work needed here... - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
Re: ThreadLocals, context listeners and classloader leaks
On 26/01/2012 17:30, Caldarale, Charles R wrote: From: Jess Holle [mailto:je...@ptc.com] Subject: Re: ThreadLocals, context listeners and classloader leaks That said, there could and arguably should be another choice: I'll suggest something more radical: define a class such as ScopeLocal where values are added to and removed from threads as they enter and leave a scope boundary. For servlet engine purposes, typical scopes would be webapp, session, request, servlet, etc. Doing it properly (and efficiently) might well require JVM intervention, but it would certainly help in providing a useful mechanism for app writers. Obviously, there's lots of definition work needed here... Imagine the fiendishly clever and machiavellian applications we'd have to debug if you did that... p THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers. -- [key:62590808] signature.asc Description: OpenPGP digital signature
RE: ThreadLocals, context listeners and classloader leaks
From: Pid [mailto:p...@pidster.com] Subject: Re: ThreadLocals, context listeners and classloader leaks Imagine the fiendishly clever and machiavellian applications we'd have to debug if you did that... Job security. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
Re: ThreadLocals, context listeners and classloader leaks
On 26/01/2012 17:48, Caldarale, Charles R wrote: From: Pid [mailto:p...@pidster.com] Subject: Re: ThreadLocals, context listeners and classloader leaks Imagine the fiendishly clever and machiavellian applications we'd have to debug if you did that... Job security. ROFL. p -- [key:62590808] signature.asc Description: OpenPGP digital signature
