Memory leak through threadlocal in NonInjectionManager

[newwdles]

The issue #5710 detected a memory leak through the usage of Thread locals in the class NonInjectionManager, it was fixed by setting the reference to the threadLocal to null.

However this way of clearing the ThreadLocal can still lead to leaks, thread locals should be cleared by calling the java.lang.ThreadLocal#remove() methods.

This SO thread explains why setting the thread local to null can still keep the value referenced.
To be honest this is a bit above my pedigree, but if i get it correctly even if the ThreadLocalMap#Entry key (the threadlocal reference) is null, the entry itself will not be garbaged collected unless the threadLocalMap backing table needs to grow.
Only then the Entry will not be copied to the new table and the GC will be able to free it. This behaviour is visible in java.lang.ThreadLocal.ThreadLocalMap#resize methods.

This issue leads to OOM in our application, it uses reactor and after using a webclient the scheduler switch's the thread to one of "http-reactor-epoll" thread, then when jersey is used the ThreadLocal map never get removed by the GC.
We use (indirectly, through another library) jersey on other threads and the MultiValueHashMap stored in the ThreadLocal do get GCed correctly, this maybe related to the low activity on the "http-reactor-epoll" thread and thus the threadlocal map doesn't grow.

[utwi]

Thanks @newwdles - any updates on this? @jansupol please have a look it seems related to #5710 and not adressed in 3.1.9 yet since i dont see changes in the mentioned class NonInjectionManager causing the leak.

We recently upgraded to Jersey Client 3.1.9 and are facing memory leaks too.
And yes we checked if responses/inputstream get closed, so we suspect a library issue.

We are use a blocking stack, not reactive. The sample code to reproduce the problem provided in #5710 does so too - so was it proven to fix the leak?

We will proceed with analyzing the heap dumps - still suspect this is affecting us since NonInjectionManager 3.1.9 is not following best practices recommended pointed out in Stackoverflow and would love to see this issue addressed.

[jansupol]

Do you define a custom @PerThread binding? Jersey uses just 4 @PerThread bindings and they relate to JAX-B objects. I am curious if that's our bindings that make the leaks...

The problem is that the beans are created per threads that Jersey has not under control, we do not know when the thread ends to remove() from the ThreadLocal, hence relying on them to be GCed...

[utwi]

The mentioned issue by newwdles was unrelated to our issue we were facing and unrelated to #5710.

Some of the client code was registering the filters instead of once on the client level in a wrong way on the WebResource level.
This has been migrated from Jersey 1.x to 3.x which lead to high CPU consumption and GC activity.

Wrong code (which seems to have worked fine in 1.x although bad practice)
WebResource r = getRestClient().resource(endpoint); r.register(someFilterInstance);

Correct way to register filters (on client level)
ClientConfig cc = new ClientConfig(); .. cc.register(MyFeature.class)

[jansupol]

@utwi That's true, registering a filter on WebTarget can trigger creating a new ClientRuntime for each registration. Doing it for each request can start a new ClientRuntime for each request. ClientRuntime is a heavy object and it is better to register the filter on the client before the ClientRuntime is instantiated and hence it can be reused for the requests started with a stored WebTarget can be reused along with the ClientRuntime and associated injection framework instance.

[jansupol]

#5811 #5813 can be a possible fix

[jansupol]

I am closing this as fixed by #5813. Jersey 3.1.10 is out. If the problem persists, feel free to file a new bug.