Do operations on ThreadLocal have to be synchronized?

Question

Here is the code I've stumbled across:

class TransactionContextHolder {

private static final ThreadLocal<TransactionContext> currentTransactionContext = new NamedInheritableThreadLocal<TransactionContext>(
    "Test Transaction Context");


static TransactionContext getCurrentTransactionContext() {
    return currentTransactionContext.get();
}

static void setCurrentTransactionContext(TransactionContext transactionContext) {
    currentTransactionContext.set(transactionContext);
}

static TransactionContext removeCurrentTransactionContext() {
    synchronized (currentTransactionContext) {
        TransactionContext transactionContext = currentTransactionContext.get();
        currentTransactionContext.remove();
        return transactionContext;
    }
}

}

The currentTransactionContext field is of type ThreadLocal and it is the only field in the class.

It seems to me that synchronization is not needed here because value stored in ThreadLocal is associated with particular thread and thus it's not a shared state. In addition I think it impacts performance as currentTransactionContext itself is shared and only one thread is allowed to enter the block while many can do it in parallel without impacting correctness.

Is the synchronization needed here?

Answer 1

In general, it's hard to make guarantees about threadsafety given only a tiny snippet of a program, since threadsafety is a property of the whole program, and synchronized can coordinate behavior across many different parts of a program.

For example: maybe there's some other piece of code somewhere else that uses crazy unsafe reflection to try to inspect and/or mutate the guts of the ThreadLocal, and that will therefore break if you mutate the ThreadLocal without locking?

Realistically, though, you are quite right: there's never any reason to synchronize on a ThreadLocal instance, except perhaps inside its initialValue method. ThreadLocal is itself a threadsafety mechanism, and it manages its threadsafety better than you could get by tacking on synchronized anyway.

(Hat-tip to Margaret Bloom for pointing out the initialValue case.)