Volatile semantic with respect to other fields

Question

Suppose I have following code

private volatile Service service;

public void setService(Service service) {
  this.service = service;
}

public void doWork() {
  service.doWork();
}

Modified field marked as volatile and its value do not depend on previous state. So, this is correct multithreaded code (don't bother about Service implementations for a minute).

As far as I know, reading volatile variable is like entering a lock, from perspective of memory visibility. It's because reading of normal variables can not be reordered with reading volatile variables.

Does this mean that following code is correct?

private volatile boolean serviceReady = false;
private Service service;

public void setService(Service service) {
  this.service = service;
  this.serviceReady = true;
}

public void doWork() {
  if ( serviceReady ) {
    service.doWork();
  }
}

Answer 1

Yes, this code is 'correct' as it stands, from Java 1.5 onwards.

Atomicity is not a concern, with or without the volatile (writes to object references are atomic), so you can cross that off the concerns list either way -- the only open question is visibility of changes and the 'correctness' of the ordering.

Any write to a volatile variable sets up a 'happens-before' relationship (the key concept of the new Java Memory Model, as specified in JSR-133) with any subsequent reads of the same variable. This means that the reading thread must have visibility into everything visible to the writing thread: that is, it must see all variables with at least their 'current' values at the time of the write.

We can explain this in detail by looking at section 17.4.5 of the Java Language Specification, specifically the following key points:

1. "If x and y are actions of the same thread and x comes before y in program order, then hb(x, y)" (that is, actions on the same thread cannot be reordered in a way to be inconsistent with program order)
2. "A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field." (this is clarifying text, explaining that write-then-read of a volatile field is a synchronization point)
3. "If hb(x, y) and hb(y, z), then hb(x, z)" (transitivity of happens-before)

So in your example:

• the write to 'service' (a) happens-before the write to 'serviceReady' (b), due to rule 1
• the write to 'serviceReady' (b) happens-before the read of same (c), due to rule 2
• therefore, (a) happens-before (c) (3rd rule)

meaning that you are guaranteed that 'service' is set correctly, in this instance, once serviceReady is true.

You can see some good write-ups using almost exactly the same example, one at IBM DeveloperWorks -- see "New Guarantees for Volatile":

values that were visible to A at the time that V was written are guaranteed now to be visible to B.

and one at the JSR-133 FAQ, written by the authors of that JSR:

Thus, if the reader sees the value true for v, it is also guaranteed to see the write to 42 that happened before it. This would not have been true under the old memory model. If v were not volatile, then the compiler could reorder the writes in writer, and reader's read of x might see 0.

Answer 2

AFAIK this is correct code.

@CPerkins: making the only the setService method synchronized won't work as you also have to synchronize on reads.

However, one variable would be enough in this case. Why do you need the extra boolean field. E.g.

private volatile Service service;

public void setService(Service service) {
  this.service = service;
}

public void doWork() {
  if ( service != null ) {
    service.doWork();
  }
}

given that no one ever calls setService to null. So you should probably a null-check:

private volatile Service service;

public void setService(Service service) {
  if (service == null) throw NullPointerException();
  this.service = service;
}

public void doWork() {
  if ( service != null ) {
    service.doWork();
  }
}