On remove() and set(null) in ThreadLocal

Related articles:

Today, when summarizing the experience of processing sonar scanning, the students in the group mentioned one point: "ThreadLocal does not call the remove() method, and there is a risk of memory leakage". Sonar is fully described as follows:

Call "remove()" on "goodsImgMapThreadLocal".

"ThreadLocal" variables should be cleaned up when no longer used

ThreadLocal variables are supposed to be garbage collected once the holding thread is no longer alive. Memory leaks can occur when holding threads are re-used which is the case on application servers using pool of threads.

To avoid such problems, it is recommended to always clean up ThreadLocal variables using the remove() method to remove the current thread's value for the ThreadLocal variable.

In addition, calling set(null) to remove the value might keep the reference to this pointer in the map, which can cause memory leak in some scenarios. Using remove is safer to avoid this issue.

Noncompliant Code Example

public class ThreadLocalUserSession implements UserSession {

private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();

public UserSession get() {
 UserSession session = DELEGATE.get();
 if (session != null) {
   return session;
 }
 throw new UnauthorizedException("User is not authenticated");
}

public void set(UserSession session) {
 DELEGATE.set(session);
}

public void incorrectCleanup() {
  DELEGATE.set(null); // Noncompliant
}

// some other methods without a call to DELEGATE.remove()
}

Compliant Solution

public class ThreadLocalUserSession implements UserSession {

private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();

public UserSession get() {
 UserSession session = DELEGATE.get();
 if (session != null) {
   return session;
 }
 throw new UnauthorizedException("User is not authenticated");
}

public void set(UserSession session) {
 DELEGATE.set(session);
}

public void unload() {
 DELEGATE.remove(); // Compliant
}

// ...
}

Exceptions

Rule will not detect non-private ThreadLocal variables, because remove() can be called from another class.

In fact, the most important thing in the whole paragraph is this sentence:

In addition, calling set(null) to remove the value might keep the reference to this pointer in the map, which can cause memory leak in some scenarios. Using remove is safer to avoid this issue.

In other words, use set(null) to delete value, but there may still be this pointer reference. Use remove to avoid this problem.

The data store in ThreadLocal is as follows:

Thread->threadLocals(ThreadLocalMap)->Entry[]->value

In fact, most of the ThreadLocal memory leaks we often talk about refer to value memory leaks: because the Entry is a soft reference to ThreadLocal, it may occur that after the ThreadLocal is GC, the value in the Entry still exists, resulting in the value being inaccessible. In addition, the thread pool is basically used now, and the threads will be reused, Therefore, there is always a strong reference to threadlocales, which will eventually lead to the risk of memory leakage. For this problem, see< Will ThreadLocal of ThreadLocal series leak memory? >It has been analyzed in, so I won't repeat it here.

Next, take a look at the remove method of ThreadLocal:

public void remove() {
    ThreadLocalMap m = getMap(Thread.currentThread());
    if (m != null)
        m.remove(this);
}

The essence is to call the remove method of ThreadLocalMap:

/**
 * Remove the entry for key.
 */
private void remove(ThreadLocal<?> key) {
    Entry[] tab = table;
    int len = tab.length;
    int i = key.threadLocalHashCode & (len-1);
    for (Entry e = tab[i];
         e != null;
         e = tab[i = nextIndex(i, len)]) {
        if (e.get() == key) {
            e.clear();
            expungeStaleEntry(i);
            return;
        }
    }
}

There is no need to struggle with the details of the remove method of ThreadLocalMap, because it can be seen from the comments, and the entries corresponding to the current ThreadLocal are directly deleted. After all entries are deleted, the referent and value in the Entry naturally belong to unreachable objects, which can certainly be used by GC.

Next, let's look at the set method of ThreadLocal:

    public void set(T value) {
        Thread t = Thread.currentThread();
        ThreadLocalMap map = getMap(t);
        if (map != null)
            map.set(this, value);
        else
            createMap(t, value);
    }

The essence is to call the set method of ThreadLocalMap:

        private void set(ThreadLocal<?> key, Object value) {

            // We don't use a fast path as with get() because it is at
            // least as common to use set() to create new entries as
            // it is to replace existing ones, in which case, a fast
            // path would fail more often than not.

            Entry[] tab = table;
            int len = tab.length;
            int i = key.threadLocalHashCode & (len-1);

            for (Entry e = tab[i];
                 e != null;
                 e = tab[i = nextIndex(i, len)]) {
                ThreadLocal<?> k = e.get();

                if (k == key) {
                    e.value = value;
                    return;
                }

                if (k == null) {
                    replaceStaleEntry(key, value, i);
                    return;
                }
            }
						
            tab[i] = new Entry(key, value);
            int sz = ++size;
            if (!cleanSomeSlots(i, sz) && sz >= threshold)
                rehash();
        }

You can see that if the value of set is null, either overwrite the value in the previous Entry or create a new Entry. Anyway, the Entry must exist. In this way, there is a risk of memory leakage. Although this risk is very low, after all, an Entry object itself will not occupy much memory even from the perspective of Retained Heap, but there are still risks.

It was also found that the famous Spring Cloud Sleuth also had such problems, which were also mentioned Issue ( https://github.com/spring-cloud/spring-cloud-sleuth/issues/27 ). It was also fixed in later versions( https://github.com/spring-cloud/spring-cloud-sleuth/commit/44e4a2d26b5e9ec63ec497f5b651b74b9bebb8ca ), also modify set(null) to remove:

Finally, to summarize, the difference between set(null) and remove is that the former only sets value to null, but the whole key value still exists, while the latter directly deletes the whole key value, so it is obviously more appropriate to use remove.

‚Äč
Welcome to the official account:

Tags: ThreadLocal

Posted on Thu, 11 Nov 2021 04:30:07 -0500 by ramtulasi