Safe publication when values are read in synchronized methods

Question

My question concerns safe publication of field values in Java (as discuessed here Java multi-threading & Safe Publication).

As I understand it, a field can be safely read (meaning access from multiple threads will see the correct value) if:

• read and write are synchronized on the same monitor
• field is final
• field is volatile

If my understanding is correct the following class should not be thread-safe since the initial value is written without these characteristics. However I find it hard to believe that I need to make first volatile even though it is only accessed from a synchronized method.

public class Foo {

    private boolean needsGreeting = true;

    public synchronized void greet() {
        if (needsGreeting) {
            System.out.println("hello");
            needsGreeting = false;
        }
    }
}

Am I missing something? Is above code correct and if so why? Or is it necessary in such cases to make first volatile or use a final AtomicBoolean or something like that in addition to accessing it from a synchronized method.

(Just to clarify, I am aware, that if the initial value was written in a synchronized method, it would be thread-safe even without the volatile keyword.)

Answer 1

There is no happens-before relationship between the end of a constructor and method invocations, and as such it is possible for one thread to start constructing the instance and make the reference available and for another thread to acquire that reference and start calling the greet() method on a partially constructed object. The synchronization in greet() does not really address that issue.

If you publish an instance via the celebrated double-checked locking pattern, it becomes easier to see how. If there was such a happens-before relationship, it should have been safe even if DCLP is used.

public class Foo {
    private boolean needsGreeting = true;

    public synchronized void greet() {
        if (needsGreeting) {
            System.out.println("Hello.");
            needsGreeting = false;
        }
    }
}

class FooUser {
    private static Foo foo;

    public static Foo getFoo() {
        if (foo == null) {
            synchronized (FooUser.class) {
                if (foo == null) {
                    foo = new Foo();
                }
            }
        }
        return foo;
    }
}

If multiple threads call FooUser.getFoo().greet() at the same time, one thread might be constructing the Foo instance, but another thread may find a non-null Foo reference prematurely, and call greet() and find needsGreeting is still false.

An example of this is mentioned in Java Concurrency in Practice (3.5).