A simple semaphore in Java

It is a simple semaphore in Java. Although it passes all their tests with flying colors, I'm still not sure it's correct.

For example, the last thing I wasn't sure about was mLock.lockInterruptibly(); vs mLock.lock(); in the method called aquire(). And I ended up using lockInterruptibly() because if the thread calling lockInterruptibly() is interrupted, then InterruptedException is thrown.

```
package edu.vuum.mocca;

import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.Condition;

/**
* @class SimpleSemaphore
*
* @brief This class provides a simple counting semaphore
* implementation using Java a ReentrantLock and a
* ConditionObject (which is accessed via a Condition). It must
* implement both "Fair" and "NonFair" semaphore semantics,
* just liked Java Semaphores.
*/
public class SimpleSemaphore {
/**
* Define a ReentrantLock to protect the critical section.
*/
// TODO - you fill in here
ReentrantLock mLock;

/**
* Define a Condition that waits while the number of permits is 0.
*/
// TODO - you fill in here
Condition mCondition;

/**
* Define a count of the number of available permits.
*/
// TODO - you fill in here. Make sure that this data member will
// ensure its values aren't cached by multiple Threads..
volatile int mAvailable;

public SimpleSemaphore(int permits, boolean fair) {
// TODO - you fill in here to initialize the SimpleSemaphore,
// making sure to allow both fair and non-fair Semaphore
// semantics.
mAvailable = permits;
mLock = new ReentrantLock(fair);
mCondition = mLock.newCondition();
}

/**
* Acquire one permit from the semaphore in a manner that can be
* interrupted.
*/
public void acquire() throws InterruptedException {
// TODO - you fill in here.
try {
mLock.lockInterruptibly();

You should put the lock statements before the try{}finally.

For example if the thread is interrupted before your interruptible acquire then lockInterruptibly will throw and the finally will try to unlock a non-locked lock (which will throw a IllegalMonitorStateException).

There is no need for the signalAll in the acquire methods, as nothing happens that may require a waiting thread to awake (all that were waiting will still need to wait).

Similarly you only need a signal() call in the release to signal a single waiting thread to awake.

Only the first issue is a true bug the other 2 are performance issues (and invalidate the fairness)

StackExchange Code Review Q#54495, answer score: 6