Hello Mark, > -----Ursprüngliche Nachricht----- > Von: Mark Eggers <its_toas...@yahoo.com.INVALID> > Gesendet: Montag, 28. März 2022 23:55 > An: users@tomcat.apache.org > Betreff: Re: AW: AW: AW: Question to possible memory leak by Threadlocal > variable > > Thomas: > > On 3/28/2022 2:01 PM, Thomas Hoffmann (Speed4Trade GmbH) wrote: > > 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 > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: users-h...@tomcat.apache.org > > > > I think I understand now, how the memory leak is created. > > So lets say Tomcat has three worker Threads W1, W2 and W3. > > If every one of them is using the CamelContext, then all of them will > > inherit > this ThreadLocal-Value within their worker threads. > > > > I will debug into the library and try to confirm this. > > > > Thanks to your explanation, the leak report makes sense to me now. > > Right now I don’t have a clue, how all the workers might release that > ThreadLocal variable. > > > > I will let you know, what the debugger says. > > > > Thanks! Thomas > > This was just released: > > https://camel.apache.org/releases/release-3.16.0/ > > And according to the release notes: > > https://issues.apache.org/jira/browse/CAMEL-17712 > > was addressed. > > Is this useful? > > . . . just my two cents > /mde/
Thanks for your message. This bug report was filed by me :) As the new version didn’t fix the memory leak completely, I investigated it further. The patch only removes the threadlocal variable form the current thread. I was on the wrong track because I thought the spawned threads by Camel created the memory leak. But as Chris explained, the worker threads get "polluted" by the threadlocal variables. Now that I understood the real cause, I will investigate the problem a bit further and try to get in touch with the Camel developers. Before reporting an update to this problem, I needed to understand the problem. Thanks to this user group, its much clearer now :) Thanks! Thomas --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
