Search code examples
javaconcurrencylockingconditional-statementsreentrantlock

Java - Reentrant Lock, can't access newly created Condition


I have created a new Condition chopstickFree and in my pickUpChopstick() method, I am waiting for a lock on it but I can't get access to it at all.

Through debugging I have found that when it gets to the chopstickFree.await() line in the pickUpChopstick() method, it just pauses indefinitely

I don't understand? That code in the constructor was just an unsure attempt to get it working but either way, I have created a new condition, signaled to all that it is free, but I can't get a lock on it at all?

public class Chopstick {
    Lock lock = new ReentrantLock();

    private Condition chopstickFree = lock.newCondition();
    private Condition chopstickInUse = lock.newCondition();

    Chopstick() {
        lock.lock();
        chopstickFree.signalAll();
        lock.unlock();
    }

    // Pick up chopstick
    public void pickUpChopstick() throws InterruptedException {
        lock.lock();

        try {
            chopstickFree.await(); // ALWAYS PAUSES HERE INDEFINITELY
            chopstickInUse.signalAll();
        } catch (InterruptedException e) {
            e.printStackTrace();
        } finally {
            lock.unlock();
        }
    }

    // Release chopstick
    public void releaseChopstick() {
        lock.lock();
        chopstickFree.signal();
        lock.unlock();
    }
}

Any ideas?

Cheers


Solution

  • Condition#signalAll() only signals threads that are currently in Condition#await() (or its friends), i.e. the signal isn't queued up for later calls.

    You need another flag protected by the lock to correctly implement:

    public class Chopstick {
      private final Lock lock = new ReentrantLock();
      private final Condition chopstickFree = lock.newCondition();
      private volatile boolean isFree = true;
    
      Chopstick() { /* Nothing */ }
    
      // Pick up chopstick
      public void pickUpChopstick() throws InterruptedException {
        lock.lock();
    
        try {
          while (!isFree) {
            chopstickFree.await();
          }
          isFree = false;
        } finally {
          lock.unlock();
        }
      }
    
      // Release chopstick
      public void releaseChopstick() {
        lock.lock();
        try {
          isFree = true;
          chopstickFree.signal();
        } finally {
          lock.unlock();
        }
      }
    }
    

    Here is a version using a Semaphore that might be a little closer in intent to your original implementation:

    public class Chopstick {
      private final Semaphore chopsticksAvailable = new Semaphore(1);
    
      Chopstick() {
        // Nothing
      }
    
      // Pick up chopstick
      public void pickUpChopstick() throws InterruptedException {
        chopsticksAvailable.acquire();
      }
    
      // Release chopstick
      public void releaseChopstick() {
        chopsticksAvailable.release();
      }
    }