Search code examples
javamultithreadingvolatilefinal

Java: Make all fields either final or volatile?

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.

Solution

  • 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