ThreadLocal and non-thread safe API

Question

So I have a non-thread safe API (some vendor software) I'm currently using, and the way we're currently using it is one object per thread. i.e. every thread has:

Foo instance = new Foo();

However, this doesn't appear to work for this particular library. Some non-thread safe bits of it still appear to butt heads, so I'm assuming this library has some static values within it. At a few points where we know that it has issues, we currently are using ReentrantLock to lock the class when need be. I.E.

public class Bar {
    protected static final ReentrantLock lock = new ReentrantLock();

    public void process() {
        Foo instance = new Foo();
        boolean locked = false;
        try{
            if(SomeCondition) {
                locked = true;
                Bar.lock.lock();
            }

            *//rest of the processing goes here

        } finally {
            if(locked){
                Bar.lock.unlock();
            }
        }
    }
}

My question is: In such an instance where the class in question is NOT thread safe, even when creating new instances of said class, is it better to use locking, or should I look i be using ThreadLocals instead? Will ThreadLocals even alleviate my actual issue? Does a ThreadLocal version of a class actually force static areas of a class to essentially be non-static?

no, ThreadLocal is not force static areas of a class to essentially be non-static — Stranger in the Q, May 25 '16 at 15:07
Why do you think a `ThreadLocal` will act differently to a local object in each thread? — Andy Turner, May 25 '16 at 15:08
@StrangerintheQ - Well, ok then, i suppose that answers my question — eerongal, May 25 '16 at 15:09
If you are already using an instance per thread, the vendor code is thread *hostile*, not merely thread unsafe. — Andy Turner, May 25 '16 at 15:09
@AndyTurner - I have never used a ThreadLocal in the past, and i couldn't find a clear answer on how it works in this particular scenario, i could just find several people recommending to use it around the web instead of synchronization blocks. So i decided to ask to see how it would work in my particular scenario — eerongal, May 25 '16 at 15:11
I'm curious what library you are actually using. Can you disclose it? — markspace, May 25 '16 at 15:16
Instead of a `ReentrantLock`, it might be easier to just wrap the class with a proxy and make all the methods `synchronized`, then use that single instance in all threads. — Andreas, May 25 '16 at 15:18
@markspace - I would prefer not to publicly disclose it, since this is all in reference to my day job for a large company with a closed source vendor, just in case. (my company requires NDAs and such for our code) — eerongal, May 25 '16 at 15:19
@Andreas - synchronizing individual methods does _not_ guarantee thread safety. just look at how iterators work in java. you have to synchronize around the entire use of the iterator in order to make the complete iteration thread safe. — jtahlborn, May 25 '16 at 15:33
You may not be able to make the class thread-safe. If it meets your minimum performance requirements with a single instance, you should be okay. If not then you have to either re-write or do without. Under extreme circumstances you COULD run the class in multiple JVMs as long as they don't have an external writable resource, but this may be more trouble than just re-writing the library yourself. With a single instance, you should synchronize it complete--never let any trace of your instance escape the synchronized block. — Bill K, May 25 '16 at 16:56

score 5 · Accepted Answer · edited May 23 '17 at 12:18

All a ThreadLocal does is create a lookup where each thread can find its own instance of an object, so that no threads have to share. Conceptually you can think of it as a map keyed by thread id.

Letting each thread use its own objects is a good strategy for some cases, it's called "thread confinement" in the JCIP book. A common example of this is that SimpleDateFormat objects were not designed to be thread-safe and concurrent threads using them generated bad results. Using a ThreadLocal lets each thread use its own DateFormat, see this question for an example.

But if your problem is that the object references static fields, then those static fields exist on the class, not on the instance, so using ThreadLocal doesn't do anything to reduce sharing.

If somehow each of your threads used its own classloader then each would have its own class and the static fields on it would not be shared. Otherwise your locking on the class seems reasonable (though probably not speedy considering all your threads would be contending for the same lock). The best approach would be working with the vendor to get them to fix their broken code.

Makes sense. Thanks for the input! And yeah, i was exploring this because locking isn't necessarily speedy, so I was hoping that maybe it could help out in this department. — eerongal, May 25 '16 at 15:22
even static locking doesn't guarantee that it will work correctly. depending on what information is being tracked through the static variables, you could still get munged information between your threads. — jtahlborn, May 25 '16 at 15:25
@jtahlborn: right, the locking granularity might have to be so broad that it makes multithreading pointless. i would be investigating the custom classloader angle. — Nathan Hughes, May 25 '16 at 15:29

score 1 · Answer 2 · edited May 25 '16 at 18:59

ThreadLocal will not solve your problem, ThreadLocal simply store different instance for each thread independently. so in your case if you have shared resource on your 3^rd party library level that wouldn't solve the problem.

A simple synchronized monitor will solve the problem, since you want to avoid concurrent access to that library, but be aware of the performance penalty of monitor - only one thread can access the lib concurrently

Just do:

public class Bar {
    private static final Object LOCK = new Object();

    public void process() {
        synchronized(LOCK) {
            Foo instance = new Foo();
            instance.myMethod();            
    }
}

ThreadLocal and non-thread safe API

2 Answers2