1

I'm trying out a simple concurrency problem from Leetcode. I've studied the topic very briefly in university, but not using the Java APIs. It seems like I can't use a ReentrantLock (or any other Lock to my knowledge) to solve the problem without running into an IllegalMonitorStateException. Yet a Semaphore (which seems like overkill, since I only need to use binary values) seems to work fine. Why is this?

Binary Semaphore vs a ReentrantLock suggests (if I understand correctly) that binary locks can only be release by the thread that acquired it, which could be the source of my issue since I am acquiring them in the constructor in my code below. Is there any other natural way to solve this problem using locks / without semaphores?

Code with Locks that raises IllegalMonitorStateException:

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

class Foo {

    private Lock printedFirst;
    private Lock printedSecond;

    public Foo() {
        this.printedFirst = new ReentrantLock();
        this.printedSecond = new ReentrantLock();
        printedFirst.lock();
        printedSecond.lock();
    }

    public void first(Runnable printFirst) throws InterruptedException {
        printFirst.run();
        printedFirst.unlock();
    }

    public void second(Runnable printSecond) throws InterruptedException {
        printedFirst.lock();
        try {
            printSecond.run();
        } finally {
            printedSecond.unlock();
        }

    }

    public void third(Runnable printThird) throws InterruptedException {
        printedSecond.lock();
        try {
            printThird.run();
        } finally {}
    }
}

Code with Semaphores that works as intended:

import java.util.concurrent.Semaphore;

class Foo {

    private Semaphore printedFirst;
    private Semaphore printedSecond;

    public Foo() {
        this.printedFirst = new Semaphore(0);
        this.printedSecond = new Semaphore(0);
    }

    public void first(Runnable printFirst) throws InterruptedException {
        printFirst.run();
        printedFirst.release();
    }

    public void second(Runnable printSecond) throws InterruptedException {
        printedFirst.acquire();
        try {
            printSecond.run();
        } finally {
            printedSecond.release();
        }

    }

    public void third(Runnable printThird) throws InterruptedException {
        printedSecond.acquire();
        try {
            printThird.run();
        } finally {}
    }
}
Christopher Rybicki
  • 369
  • 1
  • 2
  • 11
  • Can you post a full [mcve] that we can run, that shows how you are starting threads? I'm guessing your problem is that you're locking the ReentranLocks in the constructor (why?) and trying to unlock them from a different thread. If that's the case - don't do that. Only lock and unlock in the same thread. "If the current thread is not the holder of this lock then IllegalMonitorStateException is thrown." -- https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantLock.html#unlock-- – Erwin Bolwidt Jul 14 '19 at 23:51
  • re "Suggests (if I understand correctly) that binary locks can only be release by the thread that acquired it". It's unclear to me what you mean by "binary lock"; however, semaphores can be "released" by any thread. ReentrantLocks require that the acquiring thread release it. Locks and semaphores are not generally interchangeable –  Jul 15 '19 at 00:23

3 Answers3

0

The API for ReentrantLock states that unlock will throw IllegalMonitorStateException - if the current thread does not hold this lock, and in your code it looks like this is exactly what is happening.

This isn't a normal usage for ReentrantLocks, but if you wanted to accomplish this using them anyway then a way to do it might be to track state with something like:

private volatile int tracker = 0;
private Lock lock= new ReentrantLock();

public void first(Runnable printFirst) throws InterruptedException {
    lock.lock();
    try {
        printFirst.run();
        tracker++;
    } finally {
        lock.unlock();
    }
}

public void second(Runnable printSecond) throws InterruptedException {
    while(!Thread.currentThread().isInterrupted()) {
        lock.lock();
        try {
            if (tracker == 1) {
                printSecond.run();
                tracker++;
                break;
            }
        } finally {
            lock.unlock();
        }
        Thread.yield();
    }
}

public void third(Runnable printThird) throws InterruptedException {
    while(!Thread.currentThread().isInterrupted()) {
        lock.lock();
        try {
            if (tracker == 2) {
                printThird.run();
                tracker++;
                break;
            }
        } finally {
            lock.unlock();
        }
        Thread.yield();
    }
}

If you wanted this to be more efficient (less processing from the second/third functions) then you might want to look into something like Condition https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/Condition.html

As a side note you could accomplish this with only one semaphore by requiring/releasing multiple permits for each operation.

billoot
  • 77
  • 15
0

Semaphores, as you have found, are straightforward to implement and reason about - get lock, perform action, release lock. ReentrantLocks are different and are intended to serve more complicated purposes that are not present in the example you posted. Yes, ReentrantLocks are owned by a single thread and can only be released by that thread.

If your goal is to produce protected code for use by two threads, using a Semaphore would be simpler and less error-prone than a ReentrantLock. However, if your goal is to understand how ReentrantLocks work (or if your use case somehow fits better with ReentrantLocks than is clear from the example above), I would encourage you to read the Javadoc - https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantLock.html - it contains useful information about how to use them and how they behave.

Kaan
  • 5,434
  • 3
  • 19
  • 41
0

Synchronization has 2 main use cases: resource access and event passing. The concurrency problem you are trying to solve needs event passing: the second thread waits a signal from the first, and the third thread waits a signal from the second. Signals, I mean, are empty events, they carry the fact that an event has happened, without any additional details.

Semaphores are good fit for signal passing (though can be used for resource access also). ReentrantLocks were devised for resource access. Technically, any event passing mechanism is built upon resource access. So you can use ReentrantLocks for signal passing, but it requires some additional programming (demonstrated in the @billie's answer).

Alexei Kaigorodov
  • 13,189
  • 1
  • 21
  • 38