java synchronized keyword needed on primitive getter / setter method?

Question

I read some java code, and found these functions:

synchronized void setConnected(boolean connected){
   this.connected = connected;
}

synchronized boolean isConnected(){
   return connected;
}

I wonder if synchronization makes any sense here, or just author didn't understand the need for synchronized keyword?

I'd suppose that synchronized is useless here. Or am I mistaken?

Answer 1

Synchronization is needed here to prevent memory consistency errors, see http://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html. Though in this concrete case volatile would be much more efficient solution

private volatile boolean connected;

void setConnected(boolean connected){
   this.connected = connected;
}

boolean isConnected(){
   return connected;
}

Answer 2

The keyword synchronized is one way of ensuring thread-safety. Beware: there's (way) more to thread-safety than deadlocks, or missing updates because of two threads incrementing an int without synchronization.

Consider the following class:

class Connection {
  private boolean connected; 
  synchronized void setConnected(boolean connected){
    this.connected = connected;
  }
  synchronized boolean isConnected(){
    return connected;
  }
}

If multiple threads share an instance of Connection and one thread calls setConnected(true), without synchronized it is possible that other threads keep seeing isConnected() == false. The synchronized keyword guarantees that all threads sees the current value of the field.

In more technical terms, the synchronized keyword ensures a memory barrier (hint: google that).

In more details: every write made before releasing a monitor (ie, before leaving a synchronized block) is guaranteed to be seen by every read made after acquiring the same monitor (ie, after entering a block synchronizing on the same object). In Java, there's something called happens-before (hint: google that), which is not as trivial as "I wrote the code in this order, so things get executed in this order". Using synchronized is a way to establish a happens-before relationship and guarantee that threads see the memory as you would expect them to see.

Another way to achieve the same guarantees, in this case, would be to eliminate the synchronized keyword and mark the field volatile. The guarantees provided by volatile are as follows: all writes made by a thread before a volatile write are guaranteed to be visible to a thread after a subsequent volatile read of the same field.

As a final note, in this particular case it might be better to use a volatile field instead of synchronized accessors, because the two approaches provide the same guarantees and the volatile-field approach allows simultaneous accesses to the field from different threads (which might improve performance if the synchronized version has too much contention).