Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

Hello,

we are using a 3rd party lib/framework in our application.
During undeployment we see a warning about a possible memory leak:

org.apache.catalina.loader.WebappClassLoaderBase.checkThreadLocalMapForLeaks The web application [ROOT] created a ThreadLocal with key of type [java.lang.ThreadLocal.SuppliedThreadLocal] (value [java.lang.ThreadLocal$SuppliedThreadLocal@1a14329f]) and a value of type [org.apache.xxx] (value [org.apache.xxx]) 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.

I dug into the sources and found a class A which has a static ThreadLocal variable (let's call it tl). During undeployment there is a tl.remove() in the shutdown method.
The warning is logged despite that. The framework is spawning several threads which might use class A and its threadlocal variable.
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)?

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 (?)

Before filing a bug report to the maintainer of this lib, I would need to understand the cause and background of this log entry.

Thanks in advance!
Thomas

AW: AW: AW: Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

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-unsubscribe@tomcat.apache.org
> >>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

Re: AW: AW: AW: AW: Question to possible memory leak by Threadlocal variable

[Christopher Schultz <ch...@christopherschultz.net>]

Thomas,

On 3/29/22 02:42, Thomas Hoffmann (Speed4Trade GmbH) wrote:
> Hello Mark,
> 
>> -----Ursprüngliche Nachricht-----
>> Von: Mark Eggers <it...@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-unsubscribe@tomcat.apache.org
>>>>>>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: users-help@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 :)


Yes, a real fix would need to be implemented like this:

1. DefaultCamelContext has a method called cleanup() which removes the 
ThreadLocal

2. You have a Filter which dispatches own the chain and, after getting 
control back, calls DefaultCamelContext.cleanup() to remove the 
ThreadLocal just before the request processing thread moves on to other 
things.

This kind of defeats the purpose of using a ThreadLocal, IMO, because 
the point is to re-use those resources. But there isn't really a safe 
way to do that with shared threads like this.

A better way would be to use non-private classes, like java.util.Maps of 
Strings or whatever if possible.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

AW: AW: AW: AW: Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

Hello Mark,

> -----Ursprüngliche Nachricht-----
> Von: Mark Eggers <it...@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-unsubscribe@tomcat.apache.org
> >>>>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

Re: AW: AW: AW: Question to possible memory leak by Threadlocal variable

[Mark Eggers <it...@yahoo.com.INVALID>]

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-unsubscribe@tomcat.apache.org
>>>>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@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/

Re: AW: AW: AW: Question to possible memory leak by Threadlocal variable

[Christopher Schultz <ch...@christopherschultz.net>]

Thomas,

On 3/28/22 17:01, 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-unsubscribe@tomcat.apache.org
>>>>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@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.

Precisely.

> 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.

Sounds good.

And Tomcat is doing you a favor by detecting that situation and 
cleaning-out the ThreadLocals (by retiring those Threads, one at a time, 
over time) which will clean-up that mess.

so you have a short-term memory leak which will be fixed-up by a really 
nice feature in Tomcat. That feature is mostly to protect Tomcat itself 
and the other web applications deployed on it. Oh and also to prevent 
people from sending us hate-mail because their applications have a 
memory leak that they blame on Tomcat. :)

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

AW: AW: AW: Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

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-unsubscribe@tomcat.apache.org
> >>>> For additional commands, e-mail: users-help@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@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

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

Re: AW: AW: Question to possible memory leak by Threadlocal variable

[Christopher Schultz <ch...@christopherschultz.net>]

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-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: users-help@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.base/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.checkThreadLocalMapForLeaks 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-engine/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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

AW: AW: Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

> -----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-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: users-help@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.base/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 (?)
I considered the ThreadLocal class as just the manager of the thread's member variable "threadLocals".

Regarding the library:
The full log-message is:
12-Mar-2022 15:01:16.302 SCHWERWIEGEND [Thread-15] org.apache.catalina.loader.WebappClassLoaderBase.checkThreadLocalMapForLeaks 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-engine/src/main/java/org/apache/camel/impl/DefaultCamelContext.java

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.

Greetings,
Thomas



---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

Re: AW: Question to possible memory leak by Threadlocal variable

[Christopher Schultz <ch...@christopherschultz.net>]

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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@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

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org

AW: Question to possible memory leak by Threadlocal variable

["Thomas Hoffmann (Speed4Trade GmbH)" <Th...@speed4trade.com.INVALID>]

> -----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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@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.

Thanks!
Thomas

Re: Question to possible memory leak by Threadlocal variable

[Mark Thomas <ma...@apache.org>]

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org