Unfair semaphore in Java

Question

I'm tutor for a university operating systems course, and part of my work is creating exercises for them. Those exercises are mostly about concurrency and synchronization issues. For the next exercise I intend to show them the finished implementation of an unfair counting semaphore, and their job is to make a fair one. Because of a lot of factors I'm constrained to using Java as implementation language.

Though I'm fully capable of writing Java programs, it's not exactly my "mother tongue". As such, I'm probably unaware of some idioms and best practices.

As I want to give my students not just something that "works", but code that could actually be found in a production system, I hope you can give me suggestions for my implementation:

/** Simple, unfair counting semaphore.

    This class implements a simple and unfair counting semaphore.
*/
public class UnfairSemaphore implements Semaphore {
    /** Count of available permits.
     */
    protected int count;

    /** Object used to communicate with waiting threads.

        A distinct object is used herefore (and not the semaphore itself)
        to keep any signals from notify() behind the abstraction.
     */
    protected Object wire = new Object();

    /** Construct a semaphore with the given initial count.

        Sets the initial count of the newly constructed semaphore to the
        given value, which may be negative.
    */
    public UnfairSemaphore(int initial) {
        count = initial;
    }

    /** Construct a semaphore with 1 initial permit.
     */
    public UnfairSemaphore() {
        this(1);
    }

    // Implementation
    public void acquire() throws InterruptedException {
        synchronized (wire) {
            while (count <= 0) {
                wire.wait();
            }
            --count;
        }
    }

    // Implementation
    public void release() {
        synchronized (wire) {
            ++count;
            if (count > 0) {
                wire.notifyAll();
            }
        }
    }
}

Semaphore is an interface with just the two methods acquire and release.

I'm using the member wire to synchronize on and to call notifyAll and wait in order to keep the signals resulting from these "behind the abstraction", that is to make them invisible to the code using the semaphore. (I could use just notify here, couldn't I?)

I'm neither interested in making this code faster or to use different, possibly better classes (Atomic* or whatever) to implement this.

What I do want is code that correctly handles all cases. (I just don't want to give them "educational code" which shows the idea, but fails to reliably work in practice)

Answer 1

I don't like using the object as a mutex and Condition variable. Doing that would let another programmer spin up a thread to lock and never release the Semaphore.

Instead you should either provide a private Object lock = new Object(); and sync on that or use the java.util.concurrent.locks.ReentrantLock and its condition:

private final Lock mutex = new ReentrantLock();
private final Condition condition = mutex.newCondition();

public void acquire() throws InterruptedException {
    mutex.lockInterruptibly();//let interrupt throw as soon as possible
    try{
        while (count <= 0) {
            condition.await();
        }
        --count;
    } finally {
        mutex.unlock();
    }
}

// Implementation
public void release() {
    mutex.lock();
    try{
        ++count;
        if (count > 0) {
            condition.signal();
        }
    } finally {
        mutex.unlock();
    }
}

This makes it easy to change the semaphore to a fair one by passing true to the constructor.

Though I expect you will want to implement the wait queue yourself as an example.

If you want to go a step lower and no use the pre-implemented locks can instead use LockSupport which lets to park the current thread and unpark it from another thread safely.