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

Answer 3

You're right about the effects of volatile, so this should be correct, but I'm confused about your design. I don't understand why you don't just synchronize setService - it probably wouldn't be called often. If it is called more than once, the "if (serviceReady)" part is moot, since it'll still be true, but that's fine, since the replacement is atomic, if I understand correctly.

I gather that service.doWork() is thread-safe, yes?

Answer 4

In theory, it should never work. You want to insure memory consistency for two variables, and you want to rely on a volatile read on the first one. The volatile read only guarantees that a reading thread sees the most recent value of the variable. So it's definitely not as strong as entering a locked ( synchronized ) section.

In practice, it might work, depending on the implementation of volatile by the JVM you're using. If volatile reads are implemented by flushing all CPU caches, it should work. But I'm ready to bet that it will not happen. Can I force cache coherency on a multicore x86 CPU? is a good read regarding this topic.

I would say simply have a common lock ( java.util.concurrent.Lock or synchronized ) for the two variables and be done with it.

---

The Java Language Specification, Third Edition, has this to say about volatile:

8.3.1.4 volatile Fields

A field may be declared volatile, in which case the Java memory model (§17) ensures that all threads see a consistent value for the variable.

and

17.4.4 Synchronisation order

• A write to a volatile variable (§8.3.1.4) v synchronizes-with all subsequent reads of v by any thread (where subsequent is defined according to the synchronization order).
• A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field.

It says nothing about the visibility effect of other variables.