Attempt 3 at cleaning up the threadlocal leaks #206
Conversation
|
I like the improved explanations, simplified code from abandoning the try-with-resources/PoolingSupport, and robustness against circular references in the value of ETL. Going forward, I think we'll want to be careful about our use of ForkJoinPools, particularly the common pool. If an ETL is used on these pools, the thread lifetime is uncontrolled. This is particularly true for the commonPool, which may grow or shrink significantly based on other usage in the application. |
| * The standard {@link ThreadLocal} appears to be designed to be used with relatively | ||
| * short-lived Threads. Specifically, it uses a ThreadLocalMap to store ThreadLocal key/value | ||
| * Entry objects, and there are no guarantees as to when Entry references are expunged unless | ||
| * you can explicitly call remove() on the ThreadLocal instance. This means that objects |
There was a problem hiding this comment.
This means that objects referenced by ThreadLocals will not be able to GC'd for the lifetime of the Thread.
I don't think this is quite true. The inability to be GCed for the lifetime of the thread is only true if the object containing the ThreadLocal is circularly referenced by the value.
There was a problem hiding this comment.
My reasoning: TLM has a strong reference to its Entry[] and Entry has a strong reference to value. Therefore you can't GC value while the Entry exists.
static class Entry extends WeakReference<ThreadLocal<?>> {
/** The value associated with this ThreadLocal. */
Object value;
Entry(ThreadLocal<?> k, Object v) {
super(k);
value = v;
}
}
|
LGTM after license header is added to ETL. |
jbellis
force-pushed
the
ExplicitThreadLocal2
branch
from
3e55d29 to
217912f
Compare
No description provided.