Synchronized , volatile and thread safety

Question

I am reading some books about java concurrency lately. Regarding thread safety, if it is not possible to make a class inmutable, you can always ensure thread safety by synchronizing its data.

The following class would be clearly not thread safe

public class NotThreadSafe {
  private int value;

  public void setValue(int value) {
    this.value = value;
  }

  public int getValue() {
    return this.value;
  }
}

Then i can synchronize the write, but it will remain not thread safe

public class StillNotThreadSafe {
  private int value;

  public synchronized void setValue(int value) {
    this.value = value;
  }

  public int getValue() {
    return this.value;
  }
}

As i would need to synchronize not only the writes, but also the reads

public class ThreadSafe {
  private int value;

  public synchronized void setValue(int value) {
    this.value = value;
  }

  public synchronized int getValue() {
    return this.value;
  }
}

Now the question is, by using volatile I can guarantee that other threads will see the updated value, so this makes me think that this class should be thread safe

public class NotSure {
  private volatile int value;

  public synchronized void setValue(int value) {
    this.value = value;
  }

  public int getValue() {
    return this.value;
  }
}

Is the last class thread-safe??

Answer 1

Short answer:

Yes, but you don't even need synchronized in that last case. The only thing setValue does is a single operation, a write -- and volatiles are atomic within each operation. That is, each write is atomic, and each read is atomic.

Longer answer:

Of course, if you were to try to increment the value using a pattern like:

NotSure ns = new NotSure();
int v = ns.getValue();
ns.setValue(v + 1);

... then that is not thread-safe, since it involves two operations on ns.value (a read and a write), whereas volatile only gives you atomicity for one operation. In this case, even adding synchronized to both the getter and setter wouldn't be enough, since operations could be injected between both method invocations.

That last point is actually a bit of a counter-argument to the claim that "you can always ensure thread safety by synchronizing [an object's] data." If you wanted a thread-safe way to increment NotSure.value, you'd need to synchronize that whole increment operation, not just access to the object's data. And in that case, you would need to synchronize the setter, since it could otherwise interject itself between the increment method's operations. The getter still wouldn't need to be synchronized, since the volatile keyword would ensure that the getter gets either the pre-incremented or post-incremented value.