ReentrantLock Fairness

My ReentrantLock is not working as my expectiation. I expected the result of below code to have two threads locked and unlocked randomly but I got the result was always unlock on one thread. Can anyone explain ReentrantLock and I am trying to understand the fairness policy of it (i.e. new ReentrantLock(true)) and also tryLock() method. Thanks.

    final ReentrantLock lock = new ReentrantLock(true);
    new Thread(new Runnable() {
        @Override
        public void run() {
            while (true)
                try{
                    if (lock.tryLock(1, TimeUnit.SECONDS)) {
                        lock.lock();
                        System.out.println(Thread.currentThread().getName() + " locked");
                        lock.unlock();
                    } else {
                        System.out.println(Thread.currentThread().getName() + " not locked");
                    }
                    Thread.sleep(1000);
                } catch (Exception e) {
                    e.printStackTrace();
                }
        }
    }).start();
    new Thread(new Runnable() {
        @Override
        public void run() {
            while (true)
                try{
                    if (lock.tryLock(1, TimeUnit.SECONDS)) {
                        lock.lock();
                        System.out.println(Thread.currentThread().getName() + " locked");
                        lock.unlock();
                    } else {
                        System.out.println(Thread.currentThread().getName() + " not locked");
                    }
                    Thread.sleep(1000);
                } catch (Exception e) {
                    e.printStackTrace();
                }
        }
    }).start();

Results in console:

Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked
Thread-1 not locked
Thread-0 locked

Solution

Your usage is broken. You call tryLock, which may or may not lock (and the boolean it returns tells you what happened).

If tryLock worked, you immediately call lock() a second time, which is fine as it is a re-entrant lock, but you also now need to invoke unlock() twice which you aren't doing. Every call to lock() and every true-returning call to tryLock MUST be accompanied by an unlock() call.

Thus, whichever thread wins the race never releases the lock, ever, because you aren't matching your lock calls.

Given that you must unlock, there is really only one way to use locking:

someLock.lock();
try {
  do stuff here
} finally {
  someLock.unlock();
}

and with tryLock:

if (someLock.tryLock()) try {
  code goes here
} finally {
  someLock.unlock();
} else {
  code here if lock was not available
}

If your lock call isn't immediately followed by the keyword try, your code is broken. Note that the javadoc of the lock class mentions this and shows it in all relevant examples. ReentrantLock javadoc - it's right there at the top.