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

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.

Static variables are effectively shared between threads. However, the change in one thread won't be automatically be seen by other threads. Declaring the variable as volatile guarantee the visibility.

I am not sure how static volatile will be interpreted by the JVM?

Here a paste from the JVM spec that explain it all and confirm your understanding.

ACC_VOLATILE 0x0040 Declared volatile; cannot be cached.

In respective order, the parameters are flag name, value, interpretation.

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

Not at all you instantiate the object in the method scope, it will be handled as any other object.

---

More details about thread safety that might be useful

• Each thread has it own stacks.
• Local variable are added to stacks and automatically thread-safe.
• The problem with thread safety is when you try to share data between them, which is not the case here.
• If you want to share simple values between multiple thread, you might want to have a look to atomic variables instead of using the synchronized keyword.
• Whether the data is static or not make no difference, problems only come when sharing.
• When adding synchronized to a static method, the Class object will be locked.

Answer 3

Consider what the various keywords do/mean;

volatile - it guarantees that any thread that reads the field will see the most up-to-date/recently written value (this is only useful if you have mutable data you need to share)

synchronized - obtains a lock on what is synchronized so that other threads cannot do anything until the thread that has the lock is finished doing what it needs

Given that your CreateMyClass1 and CreateMyClass2 methods both re-assign _class2 to a new instance of MyClass2 it is possible for different threads to change _class2.

If you wanted to have a separate value for each thread you could consider looking at ThreadLocal. Perhaps using a static ThreadLocal might suit your needs;

private static ThreadLocal<MyClass> threadLocal = new ThreadLocal<MyClass>();

This would replace private static MyClass2 _class2;

Then in create methods you can set each threads MyClass2;

public static MyClass CreateMyClass1(String arg0 , ArrayList<MyClass3> class3) throws Exception
{
    Test myClass = new Test();        
    threadLocal.set(new MyClass2(arg0 , class3));
    return myClass;
}

All you then have to do to get each threads MyClass2 is use the threadLocal.get() method and you will have that current threads MyClass2. This removes the worry of a different thread changing the value since each thread has its own.

I hope this is somewhat helpful and please tell me if I've misinterpreted your question :)

Answer 4

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