0

I've written a Java ReadWriteLock where the readers use double-checked locking to acquire the write-lock. Is this unsafe (as is the case for DCL with lazy-instantiation)?

import java.util.concurrent.atomic.AtomicInteger;

public class DCLRWLock {
    private boolean readerAcquiringWriteLock = false;
    private boolean writerLock = false;
    private AtomicInteger numReaders = new AtomicInteger();

    public void readerAcquire() throws InterruptedException {
        while (!nzAndIncrement(numReaders)) {
            synchronized (this) {
                if (numReaders.get() != 0)
                    continue;
                if (readerAcquiringWriteLock) {
                    do {
                        wait();
                    } while (readerAcquiringWriteLock);
                } else {
                    readerAcquiringWriteLock = true;
                    writerAcquire();
                    readerAcquiringWriteLock = false;
                    assert numReaders.get() == 0;
                    numReaders.set(1);
                    notifyAll();
                    break;
                }
            }
        }
    }

    public void readerRelease() {
        if (numReaders.decrementAndGet() == 0)
            writerRelease();
    }

    public synchronized void writerAcquire() throws InterruptedException {
        while (writerLock)
            wait();
        writerLock = true;
    }

    public synchronized void writerRelease() {
        writerLock = false;
        notifyAll();
    }

    // Atomically:
    // If x is nonzero, increments x and returns true
    // Otherwise returns false
    private static boolean nzAndIncrement(AtomicInteger x) {
        for (;;) {
            int val = x.get();
            if (val == 0)
                return false;
            else if (x.compareAndSet(val, val + 1))
                return true;
        }
    }
}

I know that Java already has a ReentrantReadWriteLock. I'm more interested in the general question of how to determine what forms of DCL are or aren't safe?

dspyz
  • 5,280
  • 2
  • 25
  • 63
  • It is DCL in the sense that I first check if numReaders is zero (using the nzAndIncrement method) and then if it is, I enter a synchronized block and again immediately check that numReaders is still zero. – dspyz Aug 13 '13 at 11:26
  • 1
    Although safe, I think that this lock will be prone to writer starvation, as the readers are able to continously occupy the writer lock. – Pyranja Aug 13 '13 at 11:27

1 Answers1

5

The unsafety of the DCL comes about when we assume that just because we read a non-null reference from a shared variable, all the writes by the thread which wrote the reference will be visible. In other words, we read a reference published via a datarace and assume things will work out fine.

In your case you don't even have a data race, but just a race condition on an atomic variable. Therefore the non-safety described above certainly does not apply here.

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
  • But if DCL only occurs when a non-null is misinterpreted, then doesn't that mean a dumb work-around would be to include a boolean variable with each (possibly uninstantiated) object and set the variable to true once the object has been instantiated? Then instead of asking if(x!=null), we ask if(nonnullx). It seems like the question of whether some valid re-ordering of the code can cause unexpected results goes beyond just the case of lazy-instantiation – dspyz Aug 13 '13 at 11:37
  • No, there is no safe work around the fact that there is no guarantee of visibility in an improperly synchronized program. This is nowhere nearly as simple as you present it (with an addition of a boolean variable). – Marko Topolnik Aug 13 '13 at 11:41
  • That's my point. So it's not so much about a non-null check as it is the idea that from outside a synchronized block, you can't really say what order things are happening in. Isn't it possible that one of the reader threads in my RWLock can get a true value from nzAndIncrement even though another thread is still inside the synchronized block acquiring the write lock (ie if the compiler moves the call to numReaders.set(1) to be earlier)? – dspyz Aug 13 '13 at 11:44
  • What you are describing is a *race condition*, but not a *data race*. The term "double-checked locking" is quite narrow and specfic, and pertains to sharing objects via a data race. But maybe we digress: you want to have a thread-safe program and I explain what DCL is and isn't. You just framed your question wrong, stressing the analogy with the DCL and lazy instantiation. This *may* stem from inadequate comprehension of data races, I can't tell. – Marko Topolnik Aug 13 '13 at 11:47
  • According to wikipedia "double-checked locking (also known as "double-checked locking optimization"[1]) is a software design pattern used to reduce the overhead of acquiring a lock by first testing the locking criterion (the "lock hint") without actually acquiring the lock. Only if the locking criterion check indicates that locking is required does the actual locking logic proceed." Here my criterion is that numReaders==0. I only lock (using a synchronized statement) if numReaders==0, otherwide I don't. How does this not fit that pattern? – dspyz Aug 13 '13 at 11:57
  • In Java, the phrase "double-checked locking", especially when associated with the concept of *lazy instantiation*, has been used in the sense which I describe for a whole decade. You are welcome to use it in however broad sense you like, but you shouldn't be surprised at the misunderstanding you induce. Anyway, enough pointless digressing and good luck with your problem. – Marko Topolnik Aug 13 '13 at 12:18