Java Thread Safety of Initialized Objects

Question

Consider the following class:

public class MyClass
{
    private MyObject obj;

    public MyClass()
    {
        obj = new MyObject();
    }

    public void methodCalledByOtherThreads()
    {
        obj.doStuff();
    }
}

Since obj was created on one thread and accessed from another, could obj be null when methodCalledByOtherThread is called? If so, would declaring obj as volatile be the best way to fix this issue? Would declaring obj as final make any difference?

Edit: For clarity, I think my main question is: Can other threads see that obj has been initialized by some main thread or could obj be stale (null)?

Answer 1

For the methodCalledByOtherThreads to be called by another thread and cause problems, that thread would have to get a reference to a MyClass object whose obj field is not initialized, ie. where the constructor has not yet returned.

This would be possible if you leaked the this reference from the constructor. For example

public MyClass()
{
    SomeClass.leak(this); 
    obj = new MyObject();
}

If the SomeClass.leak() method starts a separate thread that calls methodCalledByOtherThreads() on the this reference, then you would have problems, but this is true regardless of the volatile.

Since you don't have what I'm describing above, your code is fine.

Answer 2

It depends on whether the reference is published "unsafely". A reference is "published" by being written to a shared variable; another thread reads the variable to get the reference. If there is no relationship of happens-before(write, read), the publication is called unsafe. An example of unsafe publication is through a non-volatile static field.

@chrylis 's interpretation of "unsafe publication" is not accurate. Leaking this before constructor exit is orthogonal to the concept of unsafe publication.

Through unsafe publication, another thread may observe the object in an uncertain state (hence the name); in your case, field obj may appear to be null to another thread. Unless, obj is final, then it cannot appear to be null even if the host object is published unsafely.

This is all too technical and it requires further readings to understand. The good news is, you don't need to master "unsafe publication", because it is a discouraged practice anyway. The best practice is simply: never do unsafe publication; i.e. never do data race; i.e. always read/write shared data through proper synchronization, by using synchronized, volatile or java.util.concurrent.

If we always avoid unsafe publication, do we still need final fields? The answer is no. Then why are some objects (e.g. String) designed to be "thread safe immutable" by using final fields? Because it's assumed that they can be used in malicious code that tries to create uncertain state through deliberate unsafe publication. I think this is an overblown concern. It doesn't make much sense in server environments - if an application embeds malicious code, the server is compromised, period. It probably makes a bit of sense in Applet environment where JVM runs untrusted codes from unknown sources - even then, this is an improbable attack vector; there's no precedence of this kind of attack; there are a lot of other more easily exploitable security holes, apparently.

Answer 3

This code is fine because the reference to the instance of MyClass can't be visible to any other threads before the constructor returns.

Specifically, the happens-before relation requires that the visible effects of actions occur in the same order as they're listed in the program code, so that in the thread where the MyClass is constructed, obj must be definitely assigned before the constructor returns, and the instantiating thread goes directly from the state of not having a reference to the MyClass object to having a reference to a fully-constructed MyClass object.

That thread can then pass a reference to that object to another thread, but all of the construction will have transitively happened-before the second thread can call any methods on it. This might happen through the constructing thread's launching the second thread, a synchronized method, a volatile field, or the other concurrency mechanisms, but all of them will ensure that all of the actions that took place in the instantiating thread are finished before the memory barrier is passed.

Note that if a reference to this gets passed out of the class inside the constructor somewhere, that reference might go floating around and get used before the constructor is finished. That's what's known as unsafe publishing of the object, but code such as yours that doesn't call non-final methods from the constructor (or directly pass out references to this) is fine.

Answer 4

Your other thread could see a null object. A volatile object could possibly help, but an explicit lock mechanism (or a Builder) would likely be a better solution.

Have a look at Java Concurrency in Practice - Sample 14.12

Answer 5

This class (if taken as is) is NOT thread safe. In two words: there is reordering of instructions in java (Instruction reordering & happens-before relationship in java) and when in your code you're instantiating MyClass, under some circumstances you may get following set of instructions:

• Allocate memory for new instance of MyClass;
• Return link to this block of memory;
• Link to this not fully initialized MyClass is available for other threads, they can call "methodCalledByOtherThreads()" and get NullPointerException;
• Initialize internals of MyClass.

In order to prevent this and make your MyClass really thread safe - you either have to add "final" or "volatile" to the "obj" field. In this case Java's memory model (starting from Java 5 on) will guarantee that during initialization of MyClass, reference to alocated for it block of memory will be returned only when all internals are initialized.

For more details I would strictly recommend you to read nice book "Java Concurrency in Practice". Exactly your case is described on the pages 50-51 (section 3.5.1). I would even say - you just can write correct multithreaded code without reading that book! :)

Answer 6

The originally picked answer by @Sotirios Delimanolis is wrong. @ZhongYu 's answer is correct.

There is the visibility issue of the concern here. So if MyClass is published unsafely, anything could happen.

Someone in the comment asked for evidence - one can check Listing 3.15 in the book Java Concurrency in Practice:

public class Holder { 
    private int n;
    
    // Initialize in thread A
    public Holder(int n) { this.n = n; }
    
    // Called in thread B
    public void assertSanity() { 
        if (n != n) throw new AssertionError("This statement is false."); 
    }

}

Someone comes up an example to verify this piece of code:

coding a proof for potential concurrency issue

As to the specific example of this post:

public class MyClass{
    private MyObject obj;
    
    // Initialize in thread A
    public MyClass(){
        obj = new MyObject();
    }
    
    // Called in thread B
    public void methodCalledByOtherThreads(){
        obj.doStuff();
    }
}

If MyClass is initialized in Thread A, there is no guarantee that thread B will see this initialization (because the change might stay in the cache of the CPU that Thread A runs on and has not propagated into main memory).

Just as @ZhongYu has pointed out, because the write and read happens at 2 independent threads, so there is no happens-before(write, read) relation.

To fix this, as the original author has mentioned, we can declare private MyObject obj as volatile, which will ensure that the reference itself will be visible to other threads in timely manner (https://www.logicbig.com/tutorials/core-java-tutorial/java-multi-threading/volatile-ref-object.html) .