How to Clean Up Threadlocals

How to clean up ThreadLocals

The javadoc says this:

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

If your application or (if you are talking about request threads) container uses a thread pool that means that threads don't die. If necessary, you would need to deal with the thread locals yourself. The only clean way to do this is to call the ThreadLocal.remove() method.

There are two reasons you might want to clean up thread locals for threads in a thread pool:

to prevent memory (or hypothetically resource) leaks, or
to prevent accidental leakage of information from one request to another via thread locals.

Thread local memory leaks should not normally be a major issue with bounded thread pools since any thread locals are likely to get overwritten eventually; i.e. when the thread is reused. However, if you make the mistake of creating a new ThreadLocal instances over and over again (instead of using a static variable to hold a singleton instance), the thread local values won't get overwritten, and will accumulate in each thread's threadlocals map. This could result in a serious leak.

Assuming that you are talking about thread locals that are created / used during a webapp's processing of an HTTP request, then one way to avoid the thread local leaks is to register a ServletRequestListener with your webapp's ServletContext and implement the listener's requestDestroyed method to cleanup the thread locals for the current thread.

Note that in this context you also need to consider the possibility of information leaking from one request to another.

How to clean up Java ThreadLocals in accordance with Sonar?

Sonar is right here.

Each thread will have its own ThreadLocal state and so its own instance of NumberFormat.

So in the general case it may be undesirable to not clear data from the state since the thread may be reused (recycled by the server) and the state valued for the previous client may be inconsistent for the current client.

For example some clients could have the format US, others the format FR, and so for...
Besides some threads could instantiate that ThreadLocal class, other no. But by not cleaning the state, the state will be still use memory for threads that may not need them.

Well, in your code, there is not variability of the ThreadLocal state since you set the state for any instance, so no inconsistency risk is likely, just memory "waste".

Now, I adopted the ThreadLocal approach in order to reuse NumberFormat
instances as much as possible, avoiding the creation of one instance
per call

You reuse the ThreadLocal state by a thread request basis.

So if you have 50 threads, you have 50 states.

In web applications, the server maps the client HTTP request to one thread.

So you don't create multiple instances of the formatter only in the scope of 1 http request. It means that If you use the formatter one or two time by request processing, the ThreadLocal cache doesn't bring a great value. But if you use it more, using it makes sense.

so I think if I called remove() somewhere in the code, I
would lose all the advantages of this solution

Calling remove() will not hurt performance if you do that when the request processing is done. You don't lose any advantage since you may use the formatter dozen of times in the scope of the request and it will be cleaned only at the end.

You have Request Listener in the servlet specification :
https://docs.oracle.com/javaee/7/api/javax/servlet/ServletRequestListener.html.

You could do that in void requestDestroyed(ServletRequestEvent sre).

Spring 5.x - How to cleanup a ThreadLocal entry

As you noticed, both the HandlerInterceptor and the ServletRequestListener are executed in the original servlet container thread, where the request is received. Since you are doing asynchronous processing, you need a CallableProcessingInterceptor.

Its preProcess and postProcess methods are executed on the thread where asynchronous processing will take place.

Therefore you need something like this:

WebAsyncUtils.getAsyncManager(request)//
             .registerCallableInterceptor("some_unique_key", new CallableProcessingInterceptor() {

                 @Override
                 public <T> void postProcess(NativeWebRequest request, Callable<T> task,
                         Object concurrentResult) throws Exception {
                     // remove the ThreadLocal
                 }

             });

in a method that has access to the ServletRequest and executes in the original servlet container thread, e.g. in a HandlerInterceptor#preHandle method.

Remark: Instead of registering your own ThreadLocal, you can use Spring's RequestAttributes. Use the static method:

RequestContextHolder.currentRequestAttributes()

to retrieve the current instance. Under the hood a ThreadLocal is used, but Spring takes care of setting it and removing it on every thread where the processing of your request takes place (asynchronous processing included).

Thread's ThreadLocals cleaning

Both snippets clean ThreadLocals, but they have different approach.

The longer snippet first get all thread ThreadLocals from ThreadLocalMap and call remove() method on them. You can use it only for the current thread, because remove() method just remove() value from current thread, there is no way how to specify which thread it should work with. With some modifications you could use it for any thread, you would have to call remove() on ThreadLocalMap directly.

The shorter snippet removes whole ThreadLocalMap instance from Thread. It is lazily initialized so it will be created again if needed. You can use this approach for any Thread.

I tested if all instances are removed from JVM. The test rely on GC behavior you maybe need to tune GC behaviour on some environment, but on my environment win10/OracleJava8 it pass out of the box.

Test:

@Test
public void howToCleanThreadLocalValues() throws ReflectiveOperationException {
    Thread thread = Thread.currentThread();

    // Set thread local value for current thread
    WeakReference<ThreadLocal> threadLocal = new WeakReference<>(new ThreadLocal<>());
    threadLocal.get().set("foo");

    // Get ThreadLocalMap
    Field threadLocalsField = Thread.class.getDeclaredField("threadLocals");
    threadLocalsField.setAccessible(true);
    WeakReference<Object> threadLocalMap = new WeakReference<>(threadLocalsField.get(thread));
    Assert.assertNotNull(threadLocalMap.get());
    Assert.assertNotNull(threadLocal.get());

    // Set ThreadLocalMap to null, GC do the rest
    threadLocalsField.set(Thread.currentThread(), null);
    System.gc();
    Assert.assertNull(threadLocalMap.get());
    Assert.assertNull(threadLocal.get());
}

How to identify and remove Threads/ThreadLocals initiated from our webapp in Java?

There is no solution to fix all threadlocal leaks in one go.
Normally third party libraries using Threadlocal variables have some kind of cleanup API call which can be used to clear their local thread variables.

You have to check for all reported threadlocal leaks and find the correct way of disposing them in the corresponding library. You can do this in your CustomServletContextListener

examples:

log4j (javadoc):

LogManager.shutdown()

jdbc driver: (javadoc):

DriverManager.deregisterDriver(driver);

note: Also check for new versions of your 3rd party libs to check fox fixes concerning memory leaks (and/or thread local leaks).

Is it worth cleaning ThreadLocals in Filter to solve thread pool-related issues?

I would go through the route of reporting the issue to the library developers for 2 reasons:

It will help to other people who want to use the same library, but lack the skills / time to find such a HORRIBLE memory leak.
To help the developers of the library to build a better product.

Honestly, I've never seen this type of error before and I think it's an exception rather than something that we should guard as it happens often. Could you share on which library you've seen this behaviour?

As a side note, I wouldn't mind enabling that filter in the development / test environment and logging a critical error if a ThreadLocal variable is still attached.

How to Clean Up Threadlocals