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();
            while (mAvailable == 0) {
                mCondition.await();
            }
            mAvailable--;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Acquire one permit from the semaphore in a manner that cannot be
     * interrupted.
     */
    public void acquireUninterruptibly() {
        // TODO - you fill in here.
        try {
            mLock.lock();
            while (mAvailable == 0) {
                mCondition.awaitUninterruptibly();
            }
            mAvailable--;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Return one permit to the semaphore.
     */
    void release() {
        // TODO - you fill in here.
        try {
            mLock.lock();
            mAvailable++;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Return the number of permits available.
     */
    public int availablePermits() {
        // TODO - you fill in here by changing null to the appropriate
        // return value.
        return mAvailable;
    }
}
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)