Safe construction in java multithreading

Question

This is in context of safe construction practice in java multi threading. I was reading through JCIP book, Can somebody explain me below line:

an object is in a predictable, consistent state only after its constructor returns, so publishing an object from within its constructor can publish an incompletely constructed object. This is true even if the publication is the last statement in the constructor.

Here I want to understand specific part 'This is true even if the publication is the last statement in the constructor'

I understand it is not safe to start a thread from constructor, but is it also not safe when it is last statement in constructor?

My guess is about Why it is not safe is that, JVM could do reordering of statement and last statement would not be last anymore. Please comment on this or correct my understanding.

Edit: In question what I mean by starting a thread is that leaking this reference. And this leaking could happen by multiple means like publishing an event or start of a thread.

Answer 1

Even if it is the last call in your constructor, it can lead to messy situations when you take inheritance into consideration. Here is some sample code to demonstrate the behaviour.

import java.util.concurrent.TimeUnit;

public class Test {

    public static void main(String... args) {
        A a = new B();

        try {
            TimeUnit.SECONDS.sleep(2);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        System.out.println(Thread.currentThread() + ": " + a);
    }

    public static void doSomethingWithA(final A a) {
        Thread t = new Thread(() -> {
            System.out.println(Thread.currentThread() + ": " + a);
        });

        t.start();
        try {
            t.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

class A {

    public A() {
        Test.doSomethingWithA(this);
    }

    @Override
    protected void finalize() throws Throwable {
        if (false) {
            System.out.println(MESSAGE);
        }
    }
}

class B extends A {
    boolean isConstructed = false;

    public B() {
        try {
            TimeUnit.SECONDS.sleep(1);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        this.isConstructed = true;
    }

    @Override
    public String toString() {
        return (  "B is " + (!this.isConstructed ? "not yet " : "")
                + "fully constructed.");
    }
}

Now, imagine A and B were developed by two different developers. Even though the developer of A leaked the premature object as the last statement in the constructor of A, it is not the last statement in the constructor of B (remember the implicit super() at the start of every constructor). Thus, the thread started in doSomethingWithA(...) gets access to a premature object. This is the reason why you should only call final methods of your own class in constructors. For further information, I recommend this answer.

The design of class A is inherently flawed due to the preemptive leakage of this in the constructor, see @yshavit's answer for more details.

Answer 2

There are two insights that may help you here. First, a constructor is (mostly) just like any other method, for purposes of synchronization; namely, it doesn't inherently provide any (except as noted below). And secondly, thread safety is always between individual actions.

So let's say you have the following constructor:

MyClass() {
    this.i = 123;
    MyClass.someStaticInstance = this; // publish the object
}

// and then somewhere else:
int value = MyClass.someStaticInstance.i;

The question is: what can that last expression do?

• It can throw an NullPointerException, if someStaticInstance hasn't been set yet.
• It can also result in value == 123
• But interestingly, it can also result in value == 0

The reason for that last bit is that the actions can be reordered, and a constructor isn't special in that regard. Let's take a closer look at the actions involved:

• A. allocating the space for the new instance, and setting all its fields to their default values (0 for the int i)
• B. setting <instance>.i = 123
• C. setting someStaticInstance = <instance>
• D. reading someStaticInstance, and then its i

If you reorder those a bit, you can get:

• A. allocating the space for the new instance, and setting all its fields to their default values (0 for the int i)
• C. setting someStaticInstance = <instance>
• D. reading someStaticInstance, and then its i
• B. setting <instance>.i = 123

And there you have it -- value is 0, instead of 123.

JCIP is also warning you that leaks can happen in subtle ways. For instance, let's say you don't explicitly set that someStaticInstance field, but instead merely call someListener.register(this). You've still leaked the reference, and you should assume that the listener you're registering with might do something dangerous with it, like assigning it to someStaticInstance.

This is true even if that i field is final. You get some thread safety from final fields, but only if you don't leak this from the constructor. Specifically, in JLS 17.5, it says:

An object is considered to be completely initialized when its constructor finishes. A thread that can only see a reference to an object after that object has been completely initialized is guaranteed to see the correctly initialized values for that object's final fields.

The only way to guarantee the "only see a reference to an object after that object has completely initialized" is to not leak the reference from its constructor. Otherwise, you could imagine a thread reading MyClass.someStaticInstance in the moment just after the field was set, but before the JVM recognizes the constructor as finished.

Answer 3

As an example

class MyClass {
    public MyClass() {
        publish();
    }
    void publish() {
        System.out.println((consistent() ? "" : "in") + "consistent object");
    }
    protected boolean consistent() {
        return true;
    }
}

class Child extends MyClass {
    int result;
    public Child() {
        super();
        result = 42;
    }
    protected boolean consistent() {
        return result == 42;
    }
}