javamultithreadingconcurrencyvisibilitysafe-publication

Will this AssertionError never be thrown in this case?


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();
        }
    }

Solution

  • 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.