ThreadLocal Field Never Removed
ID |
java.threadlocal_not_removed |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Java |
Tags |
reliability |
Description
Reports a private field whose declared type is java.lang.ThreadLocal (or a subclass
declared in the same file) that no remove() call anywhere in the enclosing type ever cleans
up. A field cleaned only through set(null) gets a distinct message, since that call does not
give the same guarantee as remove().
A non-private ThreadLocal field is out of scope — remove() may be called from another
class this analysis cannot see. A class that itself extends ThreadLocal is also out of scope.
A cleanup call is attributed to a field through two receiver shapes: the plain
fieldName.remove(), and the class-qualified OwningType.fieldName.remove() that a static field
is routinely cleaned through from inside its own class. The qualifier has to name the type under
examination, so another class’s same-named field never counts as this one’s cleanup. A call made
through this.fieldName, an alias or a getter is a known false negative.
Rationale
A ThreadLocal value is supposed to be reclaimed once its holding thread ends. On a server
that reuses threads from a pool, a thread never ends between requests, so a value that is never
removed lives on into the next, unrelated request that happens to reuse the same thread — a
memory leak that also risks leaking one request’s data into another’s.
set(null) looks like cleanup, but it can still leave the map entry itself reachable, keeping a
reference alive where remove() would not.
public class RequestContext {
private static final ThreadLocal<User> CURRENT_USER = new ThreadLocal<>();
public void bind(User user) {
CURRENT_USER.set(user);
}
// no call to CURRENT_USER.remove() anywhere in this class
}Remediation
Clean up with remove(), typically from a finally block around the code that used the value:
public class RequestContext {
private static final ThreadLocal<User> CURRENT_USER = new ThreadLocal<>();
public void bind(User user) {
CURRENT_USER.set(user);
}
public void unbind() {
CURRENT_USER.remove();
}
}