CODEWITHSUNDEEP
javamultithreadingconcurrencylockingmonitoring
Simon Eliasson
Simon Eliasson

Reputation: 1295

Java monitor instead of binary semaphore

I'm working on a school assignment where I'm supposed to synchronize two threads using monitors. In this case each monitor controls the access of a piece of railway and the trains need to lock that piece of railway so that the other one can't access it or has to wait until that piece of track is free. I have never used monitors before, so I'm sure it's my limited knowledge of how monitors work that is the problem. The trains and their threads themselves work perfectly, I've successfully used binary semaphores in the same code. Now I'm trying to replace the semaphores with monitors.

I basically wonder how conditions and locks works exactly. I've read on different blogs and forums but can't seem to grasp the concept.

Important note: I'm not allowed to use the synchronized keyword.

When I run the current code, i get the following error. The error occurs at occupied.signal() in the leave method: 

Exception in thread "Thread-0" java.lang.IllegalMonitorStateException at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.signalAll(AbstractQueuedSynchronizer.java:1956)

This is the code so far:

public class Monitor {

private final Lock lock = new ReentrantLock();
private final Condition occupied = lock.newCondition();

private boolean isOccupied = false;

private int id;

public Monitor(int id) {
    super();
    this.id = id;
}

public void enter(){
    lock.lock();
    try {
        if(isOccupied)
            occupied.await();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
    isOccupied = true;
}

public boolean tryEnter(){
    if(isOccupied){
        return false;
    }else{
        enter();
        return true;
    }
}

public void leave(){
    lock.unlock();
    isOccupied = false;
    occupied.signal();
}

}

I would greatly appreciate any help and/or ideas of what is wrong.

Thanks!

Upvotes: 2

Views: 580

Answers (2)

alhugone
alhugone

Reputation: 66

You don't have to implement your own monitor. Lock is Monitor:

public class Monitor {

    private final Lock lock = new ReentrantLock();

    private int id;

    public Monitor(int id) {
        super();
        this.id = id;
    }

    public void enter(){
        lock.lock();
    }

    public boolean tryEnter(){
        return lock.tryLock();
    }
    public void leave(){
         lock.unlock();
    }
}

Upvotes: 0

rolfl
rolfl

Reputation: 17707

Your locking is too coarse. As a general pattern, unless you have very exceptional circumstances, all locking should be of the form:

lock.lock();
try {
    ....
} finally {
     lock.unlock();
}

You are not using this pattern (locking and unlocking in different methods even).

Technically, your problem is that you are signalling the occupied condition when you are not holding the lock monitor.

In your program, the 'exclusive' lock on your track section is not supposed to be the actual Java Lock mechanism, but the boolean variable isOccupied. Change your code so that the two methods do a correct try...finally block, and also, you should possibly rename your Condition to be 'unoccupied', and reverse the logic of holding it.

public void enter(){
    lock.lock();
    try {
        while(isOccupied)
            unoccupied.await();
        isOccupied = true;
    } catch (InterruptedException e) {
        e.printStackTrace();
    } finally {
        lock.unlock();
    }
}


public void leave(){
    lock.lock();
    try {
        isOccupied = false;
        unoccupied.signal();
    } catch (InterruptedException e) {
        e.printStackTrace();
    } finally {
        lock.unlock();
    }
}

Upvotes: 2

Related Questions