Thread safety in java for static methods

Question

I have the following code

//EDIT :- updated code with @Riaz's answer ( this code should be thread -safe now )

public final class MyClass
{
    private static MyClass2 _class2;
    private MyClass()
    {

    }

    public static synchronized  MyClass CreateMyClass1(String arg0 , ArrayList<MyClass3> class3) throws Exception
    {
        MyClass myClass = new MyClass();        
        _class2 = new Class2(arg0 , class3);
        return myClass;
    }

    public static synchronized  MyClass CreateMyClass2(InputStream arg0 , ArrayList<MyClass3> class3) throws Exception
    {
        MyClass myClass = new MyClass();        
        _class2 = new Class2(arg0 , class3);
        return myClass;
    }

    //EDIT :- added a third method that accesses methods of the _class2 object
    public Object getSomething() //don't need synchronized for methods that don't change the state of the object
    { 
       return MyClass._class2.someMethod();
    }

    public synchronized  Object doSomethingToClass2() 
    { 
       //change state of the _class2 variable 
    }
}

I have read a few posts explaining thread safety for static methods but I have a few questions:

1. From what I understand, unless two threads can change the state of a shared mutable object, I don't need to worry about thread safety. (Assuming that I am not leaking the "this" reference.)

   So when using MyClass, can thread1 call CreateMyClass1 and thread2 call CreateMyClass2 which will mean that the _class2 variable will be changed by thread2 which is what I want to avoid. Will making _class2 as volatile prevent this? If yes, I am not sure how static volatile will be interpreted by the JVM? Will this be enough to make MyClass thread-safe?

2. Does returning an object of the MyClass class in both the static methods cause any violation of thread-safety?

Answer 1

This implementation could be a nice bad example in an exam or interview question on multi-threading!

Before answering your questions, here are three points:

I.) A final class cannot be extended. It does not at all imply that instances of the class are immutable. So it's a red herring as far the issue of multithreading here.

II.) CreateMyClass1/2 have the same implementation, violating my favorite coding principle Don't Repeat Yourself (DRY). In general you would use the keyword synchronized to prevent a method from being called simultaneously by different threads then you only need one method. The synchronized keyword prevents the _class2 variable from being interleaved by the two threads:

public static synchronized MyClass CreateMyClass(InputStream arg0 , ArrayList<MyClass3> class3) throws Exception{ MyClass myClass = new MyClass();
_class2 = new Class2(arg0 , class3); return myClass; }

III.) A tighter approach would be to use a synchronized block around the class variable, since the only shared variable you need to protect is the static _class2 variable, and it is a class resource. The local variable(s) myClass is not shared between separate threads so you do not need to fear its being interleaved:
public static MyClass CreateMyClass(InputStream arg0 , ArrayList<MyClass3> class3) throws Exception { MyClass myClass = new MyClass(); synchronized (MyClass.class){ _class2 = new Class2(arg0 , class3); }; return myClass; }

Now your questions:
1)

Unless two threads can change the state of a shared mutable object , i don't need to worry about thread safety

• correct.

So when using MyClass , can thread1 call CreateMyClass1 and thread2 call CreateMyClass2 which will mean that the _class2 variable will be changed by thread2 which is what i want to avoid. Will making _class2 as volatile prevent this? If yes , I am not sure how static volatile will be interpreted by the JVM? Will this be enough to make MyClass thread-safe?

• the static _class2 variable will be changed by subsequent invocations of the static method CreateMyClass. That is the only sensible outcome of running the code. If instead you want to avoid interleaving the assignment of _class2 to prevent it from getting into a bad state, due to the method getting called by different threads, then yes, volatile will fix this problem, and also fix your multithreading issues (as far as this snippet goes), enabling the removal of the synchronized block:
  public final class MyClass { private static volatile MyClass2 _class2; public static MyClass CreateMyClass(String arg0 , ArrayList<MyClass3> class3) throws Exception { MyClass myClass = new MyClass(); _class2 = new Class2(arg0 , class3); return myClass; } }

However, any other MyClass methods that subsequently use non-mutable methods of _class2, will not be thread-safe. Since volatile only synchronizes assignment of _class2.

2)
No. myClass is a local variable.

Answer 2

static means not associated with an instance of the containing class. This means all your objects (and static methods) share the same variable.

volatile simply means that the value may be changed by other threads without warning.

Declaring a variable as volatile (be it static or not) states that the variable will be accessed frequently by multiple threads. In Java, this boils down to instructing threads that they can not cache the variable's value, but will have to write back immediately after mutating so that other threads see the change. (Threads in Java are free to cache variables by default).

so the effect of these two modifiers are completely orthogonal. you can initialize the variable as static volatile