ReentrantReadWriteLock hang

Question

any idea why this constructor hangs indefinitely? I'm trying to create a thread-safe singleton.

private RWLockedSingleton() {
    lock.writeLock().lock();
    System.out.println("we're done!");
    isComplete = true;
    lock.writeLock().unlock();
}

BTW, I realize it may be better to place the lock in the static getter; I'm just curious to know whether it's intrinsically wrong to use the lock in a constructor

Please show a short but complete program demonstrating the problem. You might also want to ask yourself why you're using `ReentrantReadWriteLock` at all, instead of one of the various patterns which implements a thread-safe singleton without this, using the static initialization guarantees instead. — Jon Skeet, Jun 18 '12 at 14:33
It's better to do it this way: `try { lock.writeLock().lock(); // Do whatever you want to do } finally { lock.writeLock().unlock(); }` — m0skit0, Jun 18 '12 at 14:36
@jon-skeet I read about the static init method but I just wanted to make the multi-threading explicit in the code. — Eddy, Jun 18 '12 at 14:45
@Eddy: It sounds like you're adding complexity (and in a broken way, it seems) for no reason. — Jon Skeet, Jun 18 '12 at 14:46

score 3 · Answer 1 · answered Jun 18 '12 at 14:40

3

IMHO It is almost always incorrect to lock a constructor. There is a good reason you cannot add synchronized to a constructor. You should be locking in the method which calls the constructor.

You can have a writeLock() wait forever if you are holding the readLock(), even if its in the same thread.

The simplest and often the most efficient way to create a lazy loaded, thread safe singleton is to use an enum

enum Singleton {
    INSTANCE;
}

answered Jun 18 '12 at 14:40

Peter Lawrey

525,659
79
751
1,130

Waiting forever was indeed the problem I had... thanks for the tip – Eddy Jun 18 '12 at 14:48
Often the simplest solution works best. Using an enum avoids the need for an explicit lock but is still thread safe. – Peter Lawrey Jun 18 '12 at 14:50

Eddy · Answer 2 · 2012-06-18T14:47:17.110

just noticed that in the static getter I acquired a lock on the lock.readLock just before dropping in the constructor.

According to JavaDoc, if a thread has a reader lock it is not allowed to escalate to a write lock. I would have expected an Exception or Error but there may be valid uses for just stalling the thread until the readLock is unlocked. For reference, the following won't work if the constructor tries to acquire the writeLock

public static RWLockedSingleton getSingleton() {
    lock.readLock().lock();
    RWLockedSingleton ref = null;
    if (singleton == null) {
        singleton = new RWLockedSingleton();
        ref = singleton;
    }
    lock.readLock().unlock();
    return ref;
};

ReentrantReadWriteLock hang

2 Answers2