Question

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

Was it helpful?

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();
  }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top