Will this AssertionError never be thrown in this case?

Question

First off the code, from JCIP listing http://jcip.net/listings/StuffIntoPublic.java and http://jcip.net/listings/Holder.java

public class SafePublication {
    public static void main(String[] args) throws InterruptedException {
//        System.out.println(Thread.currentThread().getName());
        StuffIntoPublic t = new StuffIntoPublic();
        t.initialize();
        while (true) {
            new Thread(() -> { t.holder.assertSanity(); }).start();
        }
    }
}

//@author Brian Goetz and Tim Peierls
class StuffIntoPublic {
    public Holder holder;

    public void initialize() {
//        System.out.println(Thread.currentThread().getName());
        holder = new Holder(42);
    }
}

//@author Brian Goetz and Tim Peierls
class Holder {
    private int n;

    public Holder(int n ) {
        this.n = n;
    }

    public void assertSanity() {
        if (n != n) {
            throw new AssertionError("This statement is false.");
        }
    }
}

I am saying that the AssertionError will never be thrown in this case because of the Thread.start() happens before guarantee. Both of the System.out.printlns that are commented prints main, meaning that the main thread is which spawns all the later threads by making and calling start on the threads in the while(true) loop.

And since that is the thread that created and initialized Holder, all subsequent threads are safe to be a perfectly visible holder due to the happens-before guarantee. Am I right?

I even tried running this code for a really long time and no assertion errors.

However, if the main looked like below, then I believe it will be possible for an AssertionError

 public static void main(String[] args) throws InterruptedException {
        System.out.println(Thread.currentThread().getName());
        StuffIntoPublic t = new StuffIntoPublic();
        new Thread(() -> t.initialize() ).start();
        while (true) {
            new Thread(() -> { t.holder.assertSanity(); }).start();
        }
    }

Botheration

From where did you take SafePublication class code? – Brogan

I wrote that code – Botheration

Then I do not understand matter of your question – Brogan

Actually, SafePublication isn't safe because holder = new Holder(42); is an unsafe publication. – Darlenadarlene

In the main method of SafePublication, can an AssertionError be thrown? – Botheration

@Andrew Sure, but if this was the only code that ever used holder, is it possible for that main method to throw an AssertionError? – Botheration

@katiex7, no, new threads will see the t.holder after its proper initialisation. – Darlenadarlene

Because of the happens before guarantee of thread start right? – Botheration

Yes, you have a valid happens-before relationship when starting a thread after the initialization and never modifying the shared state. It should still mentioned that “tried running this code for a really long time and no assertion errors” is not a sufficient test, as even in the case of an improper publication, getting an AssertionError is very unlikely here, as explained here and here. – Cohligan

Answer 1

Yes, this is safe, because Thread#start guarantees happens-before. To be more wordy: any read/write to any variable that happens before Thread#start (I tend to think above in program order if you want), will also happen before any action within that Thread (it's run method).

Indeed, that could happen to break and throw that error, if there were no happens before (to allow re-orderings) and if program execution would allow those potential re-orderings. I am even inclined to say and a proper CPU used with weak memory model (assuming you are on Intel, which is a strong memory model) could increase that chance, but I am not sure.

So, as far as I can tell, the actions would take place in the following order: first the the publishing the reference is re-ordered with variable n (there is no happens-before, so this is allowed). Thread1 creates an instance of Holder. Thread2 sees that published reference and calls that method. It reads the variable n to be zero (remember that re-ordering happened and n is not yet written, thus has a default value of zero), so it then does the != check, but Thread1 that created Holder, writes n to be 12 for example before Thread2 reads it again (in the !=n part). So this can fail.

Making the value final would solve this as it introduces the right memory barriers, or happens-before rules.

Adequate