CODEWITHSUNDEEP
javamultithreadingthread-safety
IUnknown
IUnknown

Reputation: 9839

Thread safety of method

I see the following contruct for a mutable class:

public class Doubtful
{

    public static Doubtful getInstance()
    {
        DoubtfulContext doubtfulcontext;//LOCAL HEAP VARIABLE
        //...
        doubtfulcontext = new DoubtfulContext(s1, new PrincipalName(s),
            DefaultConfig.getInstance());
        //...
        doubtfulcontext.setDoubtfulStore(new DoubtfulStore(new File(s2)));
        doubtfulcontext.setKeyTab(...);
        doubtfulcontext.setSupportedEncryptionTypes(ai);
        //...
        return new Doubtful(doubtfulcontext);
    }

    // ...
}

While Doubtful may be non-mutable,but DoubtContext is definitely mutable.
Is this thread-safe?
What is the relevance of a local heap variable here?

Upvotes: 0

Views: 208

Answers (3)

Debojit Saikia
Debojit Saikia

Reputation: 10632

Local variables are confined to the executing thread.They exist on the executing thread's stack and are not accessible to other threads. And this makes the execution of getInstance method thread safe.

As you have said Doubtful is immutable, and that makes it thread safe: multiple threads can work with the same Doubtful instance without effecting others working with the same Doubtful instance. Because the threads cannot change the instance variables (Doubtful is immutable) and method local variables are confined to the executing thread.

Now DoubtfulContext is mutable and you are publishing a reference to the DoubtfulContext instance which is created locally in the method getInstance:

doubtfulcontext = new DoubtfulContext(s1, new PrincipalName(s),
        DefaultConfig.getInstance());
...
return new Doubtful(doubtfulcontext);//Publishes the reference to DoubtfulContext

which would violate the stack confinement. And there is a possibility that multiple threads can get access to the shared, mutable data of the same DoubtfulContext instance. If DoubtfulContext is a non-thread-safe object, then this would break your program.

Consider a thread T1 that invokes getInstance to get an instance of Doubtful and after that it might share the DoubtfulContext reference (that came along with Doubtful) with other threads:

1. Doubtful doubtful = Doubtful.getInstance();
2. DoubtfulContext doubtfulContext = doubtful.getDoubtfulContext();
3. new Thread(new SomeRunnable(doubtfulContext)).start();
4. doubtfulContext.chnageSomeState();

At line no 3, it creates a new thread of execution with the DoubtfulContext. Now two threads have the same DoubtfulContext. If DoubtfulContext is non-thread-safe (having non-synchronized access to instance variables), then this would break the thread safety of the program.

Upvotes: 1

Tim Boudreau
Tim Boudreau

Reputation: 1791

It's not clear what this code does to say for certain if it is the right choice. The question of thread safety is a question of what is mutable and if that object will ever be seen by more than one thread. Creating a new object that is stateful, but will only ever be seen by one thread is thread-safe. Creating an immutable object with all immutable members is thread-safe whether you return a new one or the same one repeatedly.

If you have mutable state, you have to know if the object will be seen by more than one thread. If yes, then you need to take measures to ensure that the mutable things are thread-safe.

Several options:

  • Make all of it immutable. Initialize it in a static block and store it in a static variable (I'm not really a big fan of statics - it would be cleaner and more flexible use a dependency injection framework like Guice and inject it - then the decision about whether it's a singleton or not is made at startup time).

  • If doubtfulContext is not shared, and is the only thing which is stateful - then it is stateful, but any future caller will get a new instance of it, then your method is fine. If the doubtfulContext will be passed between threads later, you may need to make that thread-safe independently

  • If you want to optimize by, say, only reading the same file once and sharing an object that represents the file, then you will need some kind of thread-safe cache

Upvotes: 0

ljgw
ljgw

Reputation: 2771

This construction looks threadsafe, if there is no method or function to access the doubtfulcontext elsewhere in the class (and if the doubtfulcontext is not modified either), and if... Basically if you use it right, it is threadsafe.

There are a lot of ifs in that sentence. It would be preferable to make the DoubtfulContext non-mutable also.

Upvotes: 0

Related Questions