Java: Make all fields either final or volatile?
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).
- You may be able to avoid synchronization for certain single field updates, e.g. if you can use an
- Also, volatile/final may be insufficient for non-thread safe values, like an
ArrayList
or an array.
If an object is shared between threads, you have two clear options:
1. Make that object read only
So, updates (or cache) have no impact.
2. Synchronize on object itself
Cache invalidation is hard. Very hard. So if you need to guarantee no stale values, you should protect that value and protect the lock around said value.
Make the lock and values as private on shared object, so the operations here are a implementation detail.
To avoid dead locks, this operations should be as "atomic", as in to avoid interact with other any lock.
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, usingvolatile
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