Re: Proposed new Java feature
On 05/27/2012 01:11 AM, Mike Schilling wrote:
First a few observations:
1. ThreadLocals may be, in general, an abomination, but there are situation
where they're the least of evils. And if you're building a framework in
which third-party code runs (e.g. a webserver), there's no way to disallow
their use.
"abomination" is a too strong word: they are a tool with particular
usefulness and particular issues. They should definitively be used
sparingly because they carry state in a kind of hidden way. But there
are good use cases (e.g. attaching transaction context to a thread).
2. ThreadLocals interact badly with ThreadPools, because the ThreadLocals
keep their value when the tyhread is put back into the pool. This can lead
to leaks and even potential security issues.
I would actually consider this good interaction with thread pools: the
local stays around for as long as the thread lives. If you introduce
security issues this way than you are probably not using thread locals
properly. There are two things that you generally need to consider with
thread locals which both result from the fact that the life time of a
thread local value extends across a current method call (i.e. earlier
and later):
1. You need to be ready to calculate the value any time because it might
be the first time that you access it in the current thread.
2. You need to be aware that the ThreadLocal value will stay around
longer than the current method call. So if you want things removed from
it after the current call terminates you better ensure it's done
(usually in a finally block).
3. The current implementation of ThreadLocal is this: each thread contains a
map from the ThreadLocal instance to its value for that thread. (This is
slightly less intiutive than giving a ThreadLocal instance a map from Thread
to value, but has the advantage that the map, which is only used within one
thread, needn't be synchronized.)
Even more important: this construction will allow for GC of all thread
local objects when the thread dies. This is important since a
ThreadLocal instance often lives much longer than threads which might
use it (especially if it is a static member which is a typical use case).
Proposed feature: a static method on Thread that clears all ThreadLocals for
the current thread. By 3 it's trivial to implement. By 1 and 2 it's
needed. And ThreadPool implementations can simply call it directly before
putting the Thread back into the pool.
I am not convinced this is a good idea: the current design ensures that
all ThreadLocals are completely independent from each other. By
introducing this clear all method you can generate side effects on other
thread locals that might not be wanted - this could at least make things
significantly more inefficient because values have to be recalculated
much more often than intended. It may in fact introduce functional
bugs: Consider a thread context which is stored in a ThreadLocal before
your current method was invoked in order to carry it forward to methods
much deeper on the call stack (e.g. a method on a JCA connection). You
decide to do Thread.clearAllLocals() in this thread. The JCA method
cannot properly deal with the TX because the thread local value is gone
and the caller relies on the ThreadLocal to be still there and when it's
gone the TX cannot be properly finished.
Side note: it happened to me more than once that I found Java's standard
library design or implementation weird. And there are in fact bad
quirks (Vector, Hashtable) but often when I think longer about how they
did it I have to say it is done properly the way they did it. So the
standard lib is definitively better than one often thinks.
Kind regards
robert