is synchronized needed in getValue() ? & volatile needed?

I’ve a class in multithreading application:

public class A {
        private volatile int value = 0; // is volatile needed here?
    
        synchronized public void increment() { 
          value++; // Atomic is better, agree
        }
    
        public int getValue() { // synchronized needed ?
          return value;
        }
    }

Answer

The keyword volatile gives you the visibility aspects and without that you may read some stale value. A volatile read adds a memory barrier such that the compiler, hardware or the JVM can’t reorder the memory operations in ways that would violate the visibility guarantees provided by the memory model. According to the memory model, a write to a volatile field happens-before every subsequent read of that same field, thus you are guaranteed to read the latest value.

The keyword synchronized is also needed since you are performing a compound action value++ which has to be done atomically. You read the value, increment it in the CPU and then write it back. All these actions has to be done atomically. However, you don’t need to synchronize the read path since the keyword volatile guarantees the visibility. In fact, use of both volatile and synchronize on the read path would be confusing and would offer no performance or safety benefit.

The use of atomic variables is generally encouraged, since they use non blocking synchronization using CAS instructions built into the CPU which yields low lock contention and higher throughput. If it were written using the atomic variables, it would be something like this.

public class A {
    private final LongAdder value = new LongAdder();

    public void increment() {
        value.add(1);
    }

    public int getValue() {
        return value.intValue();
    }
}