Does partial thread-safety make a Java class thread-safe?

Question

I came across the example below of a Java class which was claimed to be thread-safe. Could anyone please explain how it could be thread-safe? I can clearly see that the last method in the class is not being guarded against concurrent access of any reader thread. Or, am I missing something here?

public class Account {
    private Lock lock = new ReentrantLock();
    private int value = 0;
    public void increment() {
       lock.lock();
       value++;
       lock.unlock();
    }
    public void decrement() {
       lock.lock();
       value--;
       lock.unlock();
    }
    public int getValue() {
       return value;
    }
}

Answer 1

The code is not thread-safe.

Suppose that one thread calls decrement and then a second thread calls getValue. What happens?

The problem is that there is no "happens before" relationship between the decrement and the getValue. That means that there is no guarantee, that the getValue call will see the results of the decrement. Indeed, the getValue could "miss" the results of an indefinite sequence of increment and decrement calls.

Actually, unless we see the code that uses the Account class, the question of thread-safety is ill-defined. The conventional notion of thread-safety¹ of a program is about whether the code behaves correctly irrespective of thread-related non-determinacy. In this case, we don't have a specification of what "correct" behaviour is, or indeed an executable program to test or examine.

But my reading of the code² is that there is an implied API requirement / correctness criterion that getValue returns the current value of the account. That cannot be guaranteed if there are multiple threads, therefore the class is not thread-safe.

Related links:

• http://blogs.msdn.com/b/ericlippert/archive/2009/10/19/what-is-this-thing-you-call-thread-safe.aspx

---

^{1 - The Concurrency in Practice quote in @CKing's answer is also appealing to a notion of "correctness" by mentioning "invalid state" in the definition. However, the JLS sections on the memory model don't specify thread-safety. Instead, they talk about "well-formed executions".}

^{2 - This reading is supported by the OP's comment below. However, if you don't accept that this requirement is real (e.g. because it is not stated explicitly), then the flip-side is that behaviour of the "account" abstraction depends on how code outside of the Account class ... which makes this a "leaky abstraction".}

Answer 2

This is not thread safe purely due to the fact there is no guarantees about how the compiler can re-order. Since value is not volatile here is your classic example:

while(account.getValue() != 0){

}

This can be hoisted to look like

while(true){
  if(account.getValue() != 0){

  } else {
      break;
  }
}

I can imagine there are other permutations of compiler fun which can cause this to subtly fail. But accessing this getValue via multiple threads can result in failure.