0

I have read on multiple occasions that the only potentially contentious part of a ThreadLocal (with regards to synchronization) is the ThreadLocal.withInitial() call, since the initialValue() method is always called once per Thread that accesses the ThreadLocal.

So far so good. But let's say that I have the following request lifecycle enhancer:

public class MyLifecycleEnhancer extends RequestLifecycleEventHandler {

    private static ThreadLocal<Map<String, Object>> currentRequestContext = ThreadLocal.withInitial(HashMap::new);

    public void onRead(...) {
        Map<String, Object> context = currentRequestContext.get(); // calls initialValue()
        // put something in context
    }

    public void onDone(...) {
        Map<String, Object> context = currentRequestContext.get(); // does not call initialValue(), though it may be being called in another thread
        // do something with context
        currentRequestContext.remove();
    }

    // other lifecycle hooks 
}

As far as I can see, the only point of contention in this example could be in the onDone() method, as:

  1. The ThreadLocal value is queried
  2. The ThreadLocal initialValue() may be being called by another Thread.

But this doesn't make much sense to me. As soon as currentRequestContext.get() is called once (implicitly calling initialValue(), if that is the first time that get() was called), that value is stored in the current thread's ThreadLocalsMap and is only visible to that specific thread. Other threads calling initialValue() should be irrelevant in that case.

What am I missing? When, how and why should initialValue() synchronization be done?


Edit:

A specific example of the above synchronization consideration being mentioned is this SO answer, and specifically the comment by @MargaretBloom:

Exactly. The only source of race condition I can think of is the method initialValue of ThreadLocal. This is shared across all the threads and, as seen in the example in the doc linked, it must use some protection. This method is called by get since they always perform a remove after it (don't know why)

Does this synchronization concern only apply to cases where the initial value is mutable (which is not the case in my example)? Or am I misunderstanding something?

filpa
  • 3,651
  • 8
  • 52
  • 91
  • 3
    Can you tag the source where you read from? – Alanpatchi Jun 25 '20 at 12:22
  • Sure, my apologies for not doing so already. I've added the source to the question. I originally thought to add to the comment chain in the linked answer, but the answer is quite old. – filpa Jun 25 '20 at 14:21

1 Answers1

1

There should be no concerns if your withInitial supplier is returning self contained object for use the current thread only, and Hashmap::new gives a new object per thread and so is fine.

You could make things awkward if the supplier it is itself not threadsafe, or the object it returns is not threadsafe. Here are some examples of bad suppliers:

Supplier provides same map which is not threadsafe if using across threads:

static Map<String, Object> MAP = new HashMap<>();
private static ThreadLocal<Map<String, Object>> tls = ThreadLocal.withInitial(() -> MAP);

Body of supplier isn't threadsafe way to determine a unique value per thread, the get() call could return duplicates if used:

static int unique = 0;
private static ThreadLocal<String> tls = ThreadLocal.withInitial(() -> "IDENIFIER#"+(unique++));
DuncG
  • 12,137
  • 2
  • 21
  • 33