java threads synchronization

Question

In the class below, is method getIt() thread safe and why?

public class X { 
  private long myVar; 
  public void setIt(long  var){ 
    myVar = var; 
   }  
   public long getIt() { 
     return myVar; 
  } 
}

Answer 1

No, it's not. At least, not on platforms that lack atomic 64-bit memory accesses.

Suppose that Thread A calls setIt, copies 32 bits into memory where the backing value is, and is then pre-empted before it can copy the other 32 bits.

Then Thread B calls getIt.

Answer 2

It ought to be, and generally is, but is not guaranteed to be thread safe. There could be issues with different cores having different versions in CPU cache, or the store/retrieve not being atomic for all architectures. Use the AtomicLong class.

Answer 3

Since it is a read only method. You should synchronize the set method.

EDIT : I see why the get method needs to be synchronized as well. Good job explaining Phil Ross.

Answer 4

No it is not, because longs are not atomic in java, so one thread could have written 32 bits of the long in the setIt method, and then the getIt could read the value, and then setIt could set the other 32 bits.

So the end result is that getIt returns a value that was never valid.

Answer 5

It is not thread-safe. Variables of type long and double in Java are treated as two separate 32-bit variables. One thread could be writing and have written half the value when another thread reads both halves. In this situation, the reader would see a value that was never supposed to exist.

To make this thread-safe you can either declare myVar as volatile (Java 1.5 or later) or make both setIt and getIt synchronized.

Note that even if myVar was a 32-bit int you could still run into threading issues where one thread could be reading an out of date value that another thread has changed. This could occur because the value has been cached by the CPU. To resolve this, you again need to declare myVar as volatile (Java 1.5 or later) or make both setIt and getIt synchronized.

It's also worth noting that if you are using the result of getIt in a subsequent setIt call, e.g. x.setIt(x.getIt() * 2), then you probably want to synchronize across both calls:

synchronized(x)
{
  x.setIt(x.getIt() * 2);
}

Without the extra synchronization, another thread could change the value in between the getIt and setIt calls causing the other thread's value to be lost.

Answer 6

This is not thread-safe. Even if your platform guarantees atomic writes of long, the lack of synchronized makes it possible that one thread calls setIt() and even after this call has finished it is possible that another thread can call getIt() and this call could return the old value of myVar.

The synchronized keyword does more than an exclusive access of one thread to a block or a method. It also guarantees that the second thread is informed about a change of a variable.

So you either have to mark both methods as synchronized or mark the member myVar as volatile.

There's a very good explanation about synchronization here:

Atomic actions cannot be interleaved, so they can be used without fear of thread interference. However, this does not eliminate all need to synchronize atomic actions, because memory consistency errors are still possible. Using volatile variables reduces the risk of memory consistency errors, because any write to a volatile variable establishes a happens-before relationship with subsequent reads of that same variable. This means that changes to a volatile variable are always visible to other threads. What's more, it also means that when a thread reads a volatile variable, it sees not just the latest change to the volatile, but also the side effects of the code that led up the change.

Answer 7

The getter is not thread safe because it’s not guarded by any mechanism that guarantees the most up-to-date visibility. Your choices are:

• making myVar final (but then you can’t mutate it)
• making myVar volatile
• use synchronized to accessing myVar

Answer 8

AFAIK, Modern JVMs no longer split long and double operations. I don't know of any reference which states this is still a problem. For example, see AtomicLong which doesn't use synchronization in Sun's JVM.

Assuming you want to be sure it is not a problem then you can use synchronize both get() and set(). However, if you are performing an operation like add, i.e. set(get()+1) then this synchronization doesn't buy you much, you still have to synchronize the object for the whole operation. (A better way around this is to use a single operation for add(n) which is synchronized)

However, a better solution is to use an AtomicLong. This supports atomic operations like get, set and add and DOESN'T use synchronization.