Use of ThreadLocal in HK2

[glassfishrobot]

HK2 can sometimes be deployed and undeployed in an application server that uses thread pools. If this is done then the use of ThreadLocal variables in HK2 can cause memory leaks (and class leaks etc) in the application server, as the threads created by the appserver will go back into the pool and the ThreadLocals will not be cleaned up.

Therefor the use of ThreadLocal variables should be minimized except where absolutely necessary (for example in PerThread context). Some warning should be given in the PerThread cases that its use in appservers can cause memory leaks.

Affected Versions

[2.2.0]

[glassfishrobot]

Reported by @jwells131313

[glassfishrobot]

@jwells131313 said:
Here are the current set of ThreadLocals found in hk2:

$ find . -name "*.java" | xargs grep ThreadLocal
./hk2-configuration/persistence/hk2-xml-dom/hk2-config/src/main/java/org/jvnet/hk2/config/provider/internal/ConfigThreadContext.java: private static final ThreadLocal tlc = new ThreadLocal();
./hk2-configuration/persistence/hk2-xml-dom/hub-integration/src/main/java/org/glassfish/hk2/configuration/hub/xml/dom/integration/internal/ConfigListener.java: private final static ThreadLocal skipper = new ThreadLocal() {
./hk2-configuration/hk2-integration/src/main/java/org/glassfish/hk2/configuration/internal/ConfiguredByContext.java: private final static ThreadLocal<ActiveDescriptor> workingOn = new ThreadLocal>() {
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/SyncTest.java: ServiceWithThreadLocal.class,
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/SyncTest.java: ServiceWithThreadLocal threadLocal = locator.getService(ServiceWithThreadLocal.class);
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ServiceWithThreadLocal.java:public class ServiceWithThreadLocal {
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ServiceWithThreadLocal.java: private static final ThreadLocal < Boolean > upBoolean =
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ServiceWithThreadLocal.java: new ThreadLocal < Boolean > () {
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ServiceWithThreadLocal.java: private static final ThreadLocal < Boolean > downBoolean =
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ServiceWithThreadLocal.java: new ThreadLocal < Boolean > () {
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ListenerService.java: private ServiceWithThreadLocal threadService;
./hk2-runlevel/src/test/java/org/glassfish/hk2/runlevel/tests/sync/ThreadSensitiveService.java: private ServiceWithThreadLocal threadLocalService;
./hk2-locator/src/main/java/org/jvnet/hk2/internal/FactoryCreator.java: private final static ThreadLocal<HashSet<ActiveDescriptor>> recursionFinder = new ThreadLocal>>() {
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: private final static ThreadLocal<WeakHashMap<Class, String>> threadLocalAutoAnalyzerNameCache = ./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: new ThreadLocal, String>>() {
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: private final static ThreadLocal<WeakHashMap<AnnotatedElement, SoftAnnotatedElementAnnotationInfo>>
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: new ThreadLocal<WeakHashMap<AnnotatedElement, SoftAnnotatedElementAnnotationInfo>>() {
./hk2-api/src/main/java/org/glassfish/hk2/internal/InheritableThreadContext.java: private InheritableThreadLocal threadMap
./hk2-api/src/main/java/org/glassfish/hk2/internal/InheritableThreadContext.java: = new InheritableThreadLocal() {
./hk2-api/src/main/java/org/glassfish/hk2/internal/PerThreadContext.java: private ThreadLocal threadMap =
./hk2-api/src/main/java/org/glassfish/hk2/internal/PerThreadContext.java: new ThreadLocal() {

[glassfishrobot]

@jwells131313 said:
OK, removing the ones in test:

./hk2-configuration/persistence/hk2-xml-dom/hk2-config/src/main/java/org/jvnet/hk2/config/provider/internal/ConfigThreadContext.java: private static final ThreadLocal tlc = new ThreadLocal();
./hk2-configuration/persistence/hk2-xml-dom/hub-integration/src/main/java/org/glassfish/hk2/configuration/hub/xml/dom/integration/internal/ConfigListener.java: private final static ThreadLocal skipper = new ThreadLocal() {
./hk2-configuration/hk2-integration/src/main/java/org/glassfish/hk2/configuration/internal/ConfiguredByContext.java: private final static ThreadLocal<ActiveDescriptor> workingOn = new ThreadLocal>() {
./hk2-locator/src/main/java/org/jvnet/hk2/internal/FactoryCreator.java: private final static ThreadLocal<HashSet<ActiveDescriptor>> recursionFinder = new ThreadLocal>>() {
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: private final static ThreadLocal<WeakHashMap<Class, String>> threadLocalAutoAnalyzerNameCache = ./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: new ThreadLocal, String>>() {
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: private final static ThreadLocal<WeakHashMap<AnnotatedElement, SoftAnnotatedElementAnnotationInfo>>
./hk2-locator/src/main/java/org/jvnet/hk2/internal/Utilities.java: new ThreadLocal<WeakHashMap<AnnotatedElement, SoftAnnotatedElementAnnotationInfo>>() {
./hk2-api/src/main/java/org/glassfish/hk2/internal/InheritableThreadContext.java: private InheritableThreadLocal threadMap
./hk2-api/src/main/java/org/glassfish/hk2/internal/InheritableThreadContext.java: = new InheritableThreadLocal() {
./hk2-api/src/main/java/org/glassfish/hk2/internal/PerThreadContext.java: private ThreadLocal threadMap =
./hk2-api/src/main/java/org/glassfish/hk2/internal/PerThreadContext.java: new ThreadLocal() {

[glassfishrobot]

@jwells131313 said:
Changed one implementation to not use a ThreadLocal, turned the other two into non-static ThreadLocals so when the ServiceLocator is destroyed there will be no more link hard link to the ThreadLocal variable. I believe this should fix the problem

[glassfishrobot]

Marked as fixed on Wednesday, January 14th 2015, 8:36:27 am

[glassfishrobot]

morj said:
@JWells, I was under the impression that ThreadLocal value remains retained by Thread.threadLocals map forever unless you call remove() on ThreadLocal object. Because ThreadLocalMap.Entry is a only weak reference to ThreadLocal, but not the its value. And value is a hard link.

[glassfishrobot]

@jwells131313 said:
This is from the javadoc for ThreadLocal:

Each thread holds an implicit reference to its copy of a thread-local variable as long as the thread is alive and the ThreadLocal instance is accessible; after a thread goes away, all of its copies of thread-local instances are subject to garbage collection (unless other references to these copies exist).

Note the "AND". If the ThreadLocal instance is not accessible it can be garbage collected like any other variable. After all, if a ThreadLocal is no longer accessible how would anyone access those objects?

[glassfishrobot]

morj said:
The documentation says nothing about the ThreadLocal value. Yes, the ThreadLocal object is disposed, but not the value passed to it. See for youself in ThreadLocalMap.Entry.
http://java.dzone.com/articles/painless-introduction-javas-threadlocal-storage
http://www.0xcafefeed.com/2004/06/of-non-static-threadlocals-and-memory/
"First of all, the value object put into the ThreadLocal would not purge itself (garbage collected) if there are no more strong references to it."
"Java garbage collection would clean up the ThreadLocal map if the thread itself is not strongly referenced elsewhere."

[glassfishrobot]

morj said:
@JWells any considerations?

[glassfishrobot]

@jwells131313 said:
We replaced most uses of ThreadLocal with Hk2ThreadLocal, which has a way to clear out all entries on all threads

[glassfishrobot]

@saden1 said:
Question,

I'm not sure I fully understand the behavior of HK2 and thread scope inside a web container with shared thread pool. Suppose:

1. I have a thread scoped service called "MyThreadScopedService"
2. Request R1 comes for a resource called MyResource that has MyThreadScopedService as a collaborator
3. The container assigns thread T1 to request R1

What happens after the request is fulfilled? We know that thread T1 is going to go back into the container thread pool so I wonder if a subsequent request R2 is assigned thread T1 will MyResource get a new instance of MyThreadScopedService or will it get the previous instance of MyThreadScopedService?

[glassfishrobot]

@jwells131313 said:
So HK2 has no knowledge of web containers and how they use pools. It looks like what you really want is some sort of combination of thread-scope and request-scoped. It is an interesting idea. Luckily, hk2 has an API specifically designed to make that sort of scope, perhaps you should give it a try:

https://hk2.java.net/2.5.0-b05/extensibility.html#Operations

They are really useful, though sometimes it does take a little time to wrap your brain around them. Once you do you'll see nearly everything as an Operation lol at least that's what one of the guys who some work with them told me

[glassfishrobot]

@saden1 said:
Whoa! That's some evil genius shit. It looks eerily similar to what Jersey 2 does under the hood to support RequestScope.

Reading this totally made my day. Bravo! Bravo!

[glassfishrobot]

bobzilla42 said:
Related, we're running into the memory leak issue jwells fixed here by converting Hk2ThreadLocal.locals to a WeakHashMap. Could that be released?
https://java.net/projects/hk2/sources/git/revision/bb2621cdf58a7a1ba9ea739dd970c276a9002c82

[glassfishrobot]

bobzilla42 said:
Other folks running into what appears to be the same Hk2ThreadLocal.locals issue: https://java.net/jira/browse/JERSEY-3054

[glassfishrobot]

This issue was imported from java.net JIRA HK2-241