Guarding the initialization of a non-volatile field with a lock?

Question

For educational purposes I'm writing a simple version of AtomicLong, where an internal variable is guarded by ReentrantReadWriteLock. Here is a simplified example:

public class PlainSimpleAtomicLong {

  private long value;

  private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();

  public PlainSimpleAtomicLong(long initialValue) {
    this.value = initialValue;
  }

  public long get() {
    long result;

    rwLock.readLock().lock();
    result = value;
    rwLock.readLock().unlock();

    return result;
  }

  // incrementAndGet, decrementAndGet, etc. are guarded by rwLock.writeLock()
}

My question: since "value" is non-volatile, is it possible for other threads to observe incorrect initial value via PlainSimpleAtomicLong.get()? E.g. thread T1 creates L = new PlainSimpleAtomicLong(42) and shares reference with a thread T2. Is T2 guaranteed to observe L.get() as 42?

If not, would wrapping this.value = initialValue; into a write lock/unlock make a difference?

Answer 1

Chapter 17 reasons about concurrent code in terms of happens before relationships. In your example, if you take two random threads then there is no happens-before relationship between this.value = initialValue; and result = value;.

So if you have something like:

1. T1.start();
2. T2.start();
3. ...
4. T1: L = new PlainSimpleAtomicLong(42);
5. T2: long value = L.get();

The only happens-before (hb) relationships you have (apart from program order in each thread) is: 1 & 2 hb 3,4,5.

But 4 and 5 are not ordered. If however T1 called L.get() before T2 called L.get() (from a wall clock perspective) then you would have a hb relationship between unlock() in T1 and lock() in T2.

As already commented, I don't think your proposed code could break on any combination of JVM/hardware but it could break on a theoretical implementation of the JMM.

As for your suggestion to wrap the constructor in a lock/unlock, I don't think it would be enough because, in theory at least, T1 could release a valid reference (non null) to L before running the body of the constructor. So the risk would be that T2 could acquire the lock before T1 has acquired it in the constructor. There again, this is an interleaving that is probably impossible on current JVMs/hardware.

So to conclude, if you want theoretical thread safety, I don't think you can do without a volatile long value, which is how AtomicLong is implemented. volatile would guarantee that the field is initialised before the object is published. Note finally that the issues I mention here are not due to your object being unsafe (see @BrettOkken answer) but are based on a scenario where the object is not safely published across threads.