Is lazy initialization with immutable data always thread-safe?

Question

I have two classes A and B:

class A {
    private final String someData;
    private B b;

    public String getSomeData() { return someData; }

    public B getB() {
        if (b == null) {
             b = new B(someData);
        }
        return b;
    }
}

where B is immutable and computes its data only from an instance of A. A has immutable semantics, but it's internals are mutable (like hashCode in java.lang.String).

When I call getB() from two different threads, and the calls overlap, I assume each thread gets its own instance of B. But since the constructor of B gets only immutable data, the two instances of B should be equal.

Is that correct? If not, must I make getB() synchronized to make it thread-safe?

Assume that B implements equals(), which compares all instance variables of B. Same for hashCode()

When two threads call getB() at the same time it could happen that each thread has its own object B or it could also happen that they both share the same B. Question here is like @MarkElliot points out: does B override equals() and hashCode()? — morgano
– morgano, Commented Mar 9, 2014 at 23:49
@MarkElliot yes, assume that B implements equals(), which compares all instance variables of B. Same for hashCode(). — Christoph Walesch
– Christoph Walesch, Commented Mar 9, 2014 at 23:56

ruakh · Accepted Answer · 2014-03-10 00:02:27Z

7

This is not thread-safe, because you haven't created any "happens-before" relationships with volatile or synchronized, so it's possible for the two threads to interfere with each other.

The problem is that although b = new B(someData) means "allocate enough memory for an instance of B, then create the instance there, then point b to it", the system is allowed to implement it as "allocate enough memory for an instance of B, then point b to it, then create the instance" (since, in a single-threaded app, that's equivalent). So in your code, where two threads can create separate instances but return the same instance, there's a chance that one thread will return the other thread's instance before the instance is fully initialized.

answered Mar 10, 2014 at 0:02

ruakh

185k29 gold badges292 silver badges324 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Mak · Accepted Answer · 2014-03-10 09:47:01Z

0

For "But since the constructor of B gets only immutable data, the two instances of B should be equal." As you understand its not thread safe, one thread might get un-initialized B instance (B as null or inconsistent state where somedata not yet set) others might get b instance with somedata set.

To fix this you need synchronized getB method or use synchronized block with double-check lock or some non-blocking technique like AtomicReference. For you reference I am adding here sample code for how to achieve the correct threadSafe getB() method using AtomicReference.

class A {
    private final String someData = "somedata";
    private AtomicReference<B> bRef;

    public String getSomeData() { return someData; }

    public B getB() {
        if(bRef.get()== null){
            synchronized (this){
                if(bRef.get() == null)
                    bRef.compareAndSet(null,new B(someData));
            }
        }
        return bRef.get();
    }
}

class B{
    public B(String someData) {

    }
}

edited Mar 10, 2014 at 9:47

answered Mar 10, 2014 at 1:44

Mak

6125 silver badges11 bronze badges

2 Comments

ruakh Over a year ago

I'm hesitant to downvote this, because aside from its failure to initialize b (probably it should be named bRef to avoid confusion, and declared+initialized as private final AtomicReference<B> bRef = new AtomicReference<B>()), your code isn't broken . . . but it's really bizarre. Firstly, you shouldn't synchronize on B.class, because that means that all instances of A will require the same lock, which may also well be held by some other code (since A doesn't own B.class). Secondly, the compareAndSet is superfluous, since the reference is guaranteed to be null [continued]

ruakh Over a year ago

[continued] at that location; you can just write bRef.set(new B(someData)), at which point you might as well just use a volatile field. The whole point of atomics is to avoid locking; if you ever find yourself combining atomics with synchronized, you can be pretty sure that you're doing something the wrong way.

Collectives™ on Stack Overflow

Is lazy initialization with immutable data always thread-safe?

2 Answers 2

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related