Java: Make all fields either final or volatile?

Question

If I have an object which is shared between threads, it seems to me that every field should be either final or volatile, with the following reasoning:

• if the field should be changed (point to another object, update the primitive value), then the field should be volatile so that all other threads operate on the new value. Merely a synchronization on the methods which access said field is insufficient because they might return a cached value.

• if the field should never change, make it final.

However, I could not find anything about this, so I wonder whether this logic is flawed or just too obvious?

EDIT of course instead of volatile one might use a final AtomicReference or similar.

EDIT for example, see Is getter method an alternative to volatile in Java?

EDIT to avoid confusions: This question is about cache invalidation! If two threads operate on the same object, the fields of the objects can be cached (per thread), if they are not declared volatile. How can I guarantee that the cache is invalidated properly?

FINAL EDIT Thanks to @Peter Lawrey who pointed me to JLS §17 (Java memory model). As far as I see, it states that synchronization establishes a happens-before relation between operations, so that a thread sees the updates from another thread if those updates "happened-before", e.g. if getter and setter for a non-volatile field are synchronized.

Answer 1

While I feel private final should probably have been the default for fields and variables with a keyword like var making it mutable, using volatile when you don't need it is

• much slower, often around 10x slower.
• usually doesn't give you the thread safety you need, but can make finding such bugs harder by making them less likely to appear.
• unlike final which improves clarity by saying this shouldn't be altered, using volatile when it is not needed, is likely to be confusing as the reader tries to work out why it was made volatile.

if the field should be changed (point to another object, update the primitive value), then the field should be volatile so that all other threads operate on the new value.

While this is fine for reads, consider this trivial case.

volatile int x;  x++; 

This isn't thread-safe. As it's the same as

int x2 = x; x2 = x2 + 1; // multiple threads could be executing on the same value at this point. x = x2; 

What is worse is that using volatile would make this kind of bug harder to find.

As yshavit point's out, updating multiple fields is harder to work around with volatile e.g. HashMap.put(a, b) updates multiple references.

Merely a synchronization on the methods which access said field is insufficient because they might return a cached value.

synchronized gives you all the memory guarantees of volatile and more, which is why it's significantly slower.

NOTE: Just synchronized-ing every method isn't always enough either. StringBuffer has every method synchronized but is worst than useless in a multi-threaded context as it's use is likely to be error-prone.

It's too easy to assume that achieving thread safety is like sprinkling fairy dust, add some magic thread safety and your bugs go away. The problem is that thread safety is more like a bucket with many holes. Plug the biggest holes and the bugs can appear to go away, but unless you plug them all, you don't have thread safety, but it can be harder to find.

In terms of synchronized vs volatile, this states

Other mechanisms, such as reads and writes of volatile variables and the use of classes in the java.util.concurrent package, provide alternative ways of synchronization.

https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html

Answer 2

Making fields you don't need to change final is a good idea, irrespective of threading concerns. It makes instances of the class easier to reason about, because you can know what state it is in more easily.

In terms of making the other fields volatile:

Merely a synchronization on the methods which access said field is insufficient because they might return a cached value.

You would only see a cached value if you accessing the value outside a synchronized block.

All accesses would need to be correctly synchronized. The end of one synchronized block is guaranteed to happen before the start of another synchronized block (when synchronizing on the same monitor).

There are at least a couple of cases where you would still need to use synchronization:

• You would want to use synchronization if you had to read and then update one or more fields atomically.
  • You may be able to avoid synchronization for certain single field updates, e.g. if you can use an Atomic* class instead of a "plain old field"; but even for a single field update, you could still require exclusive access (e.g. adding one element to a list whilst removing another).
• Also, volatile/final may be insufficient for non-thread safe values, like an ArrayList or an array.