Hello Chris, > -----Ursprüngliche Nachricht----- > Von: Christopher Schultz <ch...@christopherschultz.net> > Gesendet: Montag, 28. März 2022 18:48 > An: users@tomcat.apache.org > Betreff: Re: AW: AW: Question to possible memory leak by Threadlocal > variable > > Thomas, > > On 3/25/22 16:59, Thomas Hoffmann (Speed4Trade GmbH) wrote: > >> -----Ursprüngliche Nachricht----- > >> Von: Christopher Schultz <ch...@christopherschultz.net> > >> Gesendet: Freitag, 25. März 2022 14:05 > >> An: users@tomcat.apache.org > >> Betreff: Re: AW: Question to possible memory leak by Threadlocal > >> variable > >> > >> Thomas, > >> > >> On 3/24/22 05:49, Thomas Hoffmann (Speed4Trade GmbH) wrote: > >>> > >>> > >>>> -----Ursprüngliche Nachricht----- > >>>> Von: Mark Thomas <ma...@apache.org> > >>>> Gesendet: Donnerstag, 24. März 2022 09:32 > >>>> An: users@tomcat.apache.org > >>>> Betreff: Re: Question to possible memory leak by Threadlocal > >>>> variable > >>>> > >>>> On 24/03/2022 07:57, Thomas Hoffmann (Speed4Trade GmbH) wrote: > >>>> > >>>> <snip/> > >>>> > >>>>> Is it correct, that every spawned thread must call tl.remove() to > >>>>> cleanup all > >>>> the references to prevent the logged warning (and not only the main > >>>> thread)? > >>>> > >>>> Yes. Or the threads need to exit. > >>>> > >>>>> Second question is: How might it cause a memory leak? > >>>>> The threads are terminated and hold a reference to this static > >>>>> variable. But > >>>> on the other side, that class A is also eligible for garbage > >>>> collection after undeployment. > >>>>> So both, the thread class and the class A are ready to get garbage > >>>>> collected. Maybe I missed something (?) > >>>> > >>>> It sounds as if the clean-up is happening too late. Tomcat expects > >>>> clean-up to be completed once contextDestroyed() has returned for > >>>> all ServLetContextListeners. If the clean-up is happening > >>>> asynchronously > >> (e.g. > >>>> the call is made to stop the threads but doesn't wait until the > >>>> threads have > >>>> stopped) you could see this message. > >>>> > >>>> In this case it sounds as if you aren't going to get a memory leak > >>>> but Tomcat can't tell that at the point it checks. > >>>> > >>>> Mark > >>>> > >>>> ------------------------------------------------------------------- > >>>> -- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org > >>>> For additional commands, e-mail: users-h...@tomcat.apache.org > >>> > >>> Hello Mark, > >>> thanks for the information. > >>> The shutdown of the framework is currently placed within the > >>> destroy() > >> method of a servlet (with load on startup). > >>> At least the debugger shows that servlet-->destroy() is executed > >>> before > >> the method checkThreadLocalMapForLeaks() runs. > >>> I will take a look, whether the threads already exited. > >> > >> Tomcat only checks its own request-processing threads for > >> ThreadLocals, so any threads created by the application or that > >> library are unrelated to the warning you are seeing. > >> > >> Any library which saves ThreadLocals from request-processing threads > >> is going to have this problem if the objects are of types loaded by > >> the webapp ClassLoader. > >> > >> There are a few ways to mitigate this, but they are ugly and it would > >> be better if the library didn't use ThreadLocal storage, or if it > >> would use vanilla classes from java.* and not its own types. > >> > >> You say that those objects are eligible for GC after the library > >> shuts down, but that's not true: anything you stick in ThreadLocal storage > is being held ... > >> by the ThreadLocal storage and won't be GC'd. If an object can't be > >> collected, the java.lang.Class defining it can't be collected, and > >> therefore the ClassLoader which loaded it (the webapp > >> ClassLoader) can't be free'd. We call this a "pinned ClassLoader" and > >> it still contains all of the java.lang.Class instances that the > >> ClassLoader ever loaded during its lifetime. If you reload > >> repeatedly, you'll see un-collectable ClassLoader instances piling up > >> in memory which is > >> *definitely* a leak. > >> > >> The good news for you is that Tomcat has noticed the problem and > >> will, over time, retire and replace each of the affected Threads in > >> its request- processing thread pool. As those Thread objects are > >> garbage-collected, the TheradLocal storage for each is also > >> collected, etc. and *eventually* your leak will be resolved. But it would > >> be > better not to have one in the first place. > >> > >> Why not name the library? Why anonymize the object type if it's > >> org.apache.something? > >> > >> -chris > > > > Hello Chris, > > I didn't want to blame any library 😉 But as you ask for it, I send more > details. > > > > Regarding the ThreadLocal thing: > > I thought that the threadlocal variables are stored within the > > Thread-class in the member variable "ThreadLocal.ThreadLocalMap > > threadLocals": > > https://github.com/AdoptOpenJDK/openjdk- > jdk11/blob/master/src/java.bas > > e/share/classes/java/lang/Thread.java > > > > So I thought, when the thread dies, these variables will also be > > released and automatically removed from the ThreadLocal variable / > > instance (?) > This is correct, but if the ThreadLocal is being stored in the request- > processing thread, then when your web application is redeployed, the > request processing threads outlive that event. Maybe you thought your > application gets a private set of threads for its own use, but that's not the > case: Tomcat pools threads across all applications deployed on the server. > You can play some games to segregate some applications from others, but > it's a lot of work for not much gain IMO. > > Since the threads outlive the application, you can see the problem, now. > > > I considered the ThreadLocal class as just the manager of the thread's > > member variable "threadLocals". > Basically, yes. > > > Regarding the library: > > The full log-message is: > > 12-Mar-2022 15:01:16.302 SCHWERWIEGEND [Thread-15] > org.apache.catalina.loader.WebappClassLoaderBase.checkThreadLocalMapF > orLeaks The web application [ROOT] created a ThreadLocal with key of type > [java.lang.ThreadLocal.SuppliedThreadLocal] (value > [java.lang.ThreadLocal$SuppliedThreadLocal@2121cbad]) and a value of type > [org.apache.camel.impl.DefaultCamelContext.OptionHolder] (value > [org.apache.camel.impl.DefaultCamelContext$OptionHolder@338d0413]) > but failed to remove it when the web application was stopped. Threads are > going to be renewed over time to try and avoid a probable memory leak. > > > > The blamed class is this version: > > https://github.com/apache/camel/blob/camel-3.14.x/core/camel-core- > engi > > ne/src/main/java/org/apache/camel/impl/DefaultCamelContext.java > > Interesting that Camel is storing a ThreadLocal. Maybe there is a better way > to use Camel in the context of a web application? > > > Within our app we have a startup servlet with: > > Servlet-> init: context = new DefaultCamelContext(); > > Servlet -> destroy: context.stop(); > > > > The stop-method will call the doStop() method of this class (via the > > class hierarchy DefaultCamelContext --> SimpleCamelContext --> > > AbstractCamelContext). > After the destroy-method is executed, all > > spawned threads of Camel are stopped / vanished. There is also no log > > entry, that some orphaned threads exist when undeploying the app. > > > > So I don’t know, what's the mistake within this class. What would be > > the right way to clean up the ThreadLocal variable? Just stopping the > > threads didn’t seem to clean it up properly. > The saved ThreadLocal was done from within one of the request-processing > threads that Tomcat owns. This wasn't a thread spawned by the library, > which is likely to already be cleaned-up when stop() is completed, as > expected. > > It looks like Camel may be capturing some values and storing them in > ThreadLocal when it doesn't make sense (in a web application) to do so. > > Are you able to instrument your application to see when those ThreadLocals > are set? > > -chris >
I debugged the application with the library. The leak already happens, because during startup of Tomcat, the main thread is running the startup-servlet (init). Whereas when I remove the war, a thread called "Catalina-utility-2" is shutting down the application and runs the destroy() method. Thus the shutdown-method and the startup-method are executed by two different threads. Thus the cleanup of the ThreadLocal already fails therefore. I will try to address the issue to the project team of Camel. Thanks for shifting me to the right track! Greetings, Thomas --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
