Thomas,
On 3/29/22 02:42, Thomas Hoffmann (Speed4Trade GmbH) wrote:
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 :)
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
