Re: AW: AW: AW: AW: Question to possible memory leak by Threadlocal variable
Thomas, On 3/29/22 02:42, Thomas Hoffmann (Speed4Trade GmbH) wrote: Hello Mark, -Ursprüngliche Nachricht- Von: Mark Eggers 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 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 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 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: 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- proce
AW: AW: AW: Question to possible memory leak by Threadlocal variable
Hello Chris, > -Ursprüngliche Nachricht- > Von: Christopher Schultz > 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 > >> 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 > >>>> 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: > >>>> > >>>> > >>>> > >>>>> 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 > >>
AW: AW: AW: AW: Question to possible memory leak by Threadlocal variable
Hello Mark, > -Ursprüngliche Nachricht- > Von: Mark Eggers > 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 > >> 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 > >>>> 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 > >>>>>> 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: > >>>>>> > >>>>>> > >>>>>> > >>>>>>> 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 wh
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 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 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 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: 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.
Re: AW: AW: AW: Question to possible memory leak by Threadlocal variable
Thomas, On 3/28/22 17:01, Thomas Hoffmann (Speed4Trade GmbH) wrote: Hello Chris, -Ursprüngliche Nachricht- Von: Christopher Schultz 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 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 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: 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.
AW: AW: AW: Question to possible memory leak by Threadlocal variable
Hello Chris, > -Ursprüngliche Nachricht- > Von: Christopher Schultz > 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 > >> 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 > >>>> 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: > >>>> > >>>> > >>>> > >>>>> 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 > >>
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 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 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: 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.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&qu
AW: AW: Question to possible memory leak by Threadlocal variable
> -Ursprüngliche Nachricht- > Von: Christopher Schultz > 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 > >> 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: > >> > >> > >> > >>> 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.base/share/classes/java/lang/Thread.java
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 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: 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 - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
AW: Question to possible memory leak by Threadlocal variable
> -Ursprüngliche Nachricht- > Von: Mark Thomas > 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: > > > > > 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. Thanks! Thomas
