java.lang.IllegalMonitorStateException in Monitor class of dining philosophers

Question

I am new to monitors and condition variables. I am using lock and condition variables in my monitor.

public class Monitor  
{   
    private final int piNumberOfPhilosophers;
    private PhilosopherCard[] self;
    private Integer[] names;
    private int invited = 0;
    static Lock lock = new ReentrantLock();
    private Condition[] status; // = lock.newCondition();
    private String[] state;
    /**
     * Constructor
     */
    public Monitor(int piNumberOfPhilosophers)
    {        this.piNumberOfPhilosophers = piNumberOfPhilosophers;         

        self = new PhilosopherCard[this.piNumberOfPhilosophers];
        names = new Integer[this.piNumberOfPhilosophers];
        status = new Condition [this.piNumberOfPhilosophers];
        state = new String [this.piNumberOfPhilosophers];
        //Arrays.asList(state).indexOf(4);      
        }

    public void invitePhilosopher (int id){

        names[invited] = id;
        System.out.println(invited);
        PhilosopherCard philosopher = new PhilosopherCard("thinking");
        self[invited] = philosopher;
        status[invited] =lock.newCondition();
        state[invited] = "thinking";
        invited++;
        }           
    /**
     * check other philosophers (<_<) - > (0_o) -> (>_>)
     */

    private void  test (int index){
        lock.lock();

        int left = index-1;
        int right = index +1;
        if(index==0){
            left=piNumberOfPhilosophers-1;
        }
        if(index == piNumberOfPhilosophers-1){
            right = 0;
        }
        if((state[left]!="eating")&(state[right]!="eating")){
            state[index]="eating";
            status[index].signal();
            }
        lock.unlock();

    }


    public void pickUp(final int piTID) throws InterruptedException
    {        
        int index = Arrays.asList(names).indexOf(piTID);    
        state[index]="hungry";
        test(index);
        if(!state[index].equals("eating")){     
                status[index].wait();
        }   
    }

    /**
     * When a given philosopher's done eating, they put the chopstiks/forks down
     * and let others know they are available.
     */
    public void putDown(final int piTID)
    {   
        int index = Arrays.asList(names).indexOf(piTID);
        self[index].setState("thinking");

        int left = index-1;
        int right = index +1;
        if(index==0){
            left=piNumberOfPhilosophers-1;
        }
        if(index == piNumberOfPhilosophers-1){
            right = 0;
        }
        test(left);
        test(right);

        // ...
    }


}

In putdown we can self[index].signal to wake up monitors. But it is not that important. And, in pick up method monitor exception occurs, when we use wait on condition variable. Why? Because they all use 1 lock? All trace

Exception in thread "Thread-1" Exception in thread "Thread-3" java.lang.IllegalMonitorStateException
    at java.lang.Object.wait(Native Method)
    at java.lang.Object.wait(Unknown Source)
    at Monitor.pickUp(Monitor.java:75)
    at Philosopher.run(Philosopher.java:95)
java.lang.IllegalMonitorStateException
    at java.lang.Object.wait(Native Method)
    at java.lang.Object.wait(Unknown Source)
    at Monitor.pickUp(Monitor.java:75)
    at Philosopher.run(Philosopher.java:95)

I updated the code and removed extra class, so all in one class, maybe now it is more clear where can be that error

Answer 1

There are a number of things you're doing wrong.

1. You are synchronizing on this and not locking on PhilosopherCard.lock. By locking, I mean PhilosopherCard.lock.lock();
2. You are using wait instead of await.

---

Update for more information

If you take a look at this code of yours and remove synchronized the code won't fail.

   private void test (int index){
        PhilosopherCard.lock.lock();
        int left = index-1;
        int right = index +1;
        if(index==0){
            left=piNumberOfPhilosophers-1;
        }
        if(index == piNumberOfPhilosophers-1){
            right = 0;
        }
        if((state[left]!="eating")&(state[right]!="eating")){
            state[index]="eating";
            status[index].signal();;
            }
        PhilosopherCard.lock.unlock();
    }

Where you signal it is similar to await, but without synchronized why wouldn't it throw an IMSE? That's because you are holding the PhilosopherCard.lock lock. If you removed those two locks you would get an IMSE.

You are running into that issue in pickUp. I would remove synchronized from the method all together. Why? Because you are mixing synchronization. If you want to do synchronization with synchronized that's fine, but if you're doing synchronization with java.util.concurrent.Lock then you cannot use synchronized.

The synchronized keyword can allow you to use wait, notify and notifyAll on the object.

The j.u.c.Lock and j.u.c.Condition allow you to use await, signal, signalAll. So my suggestion is either use only Lock/Condition or synchronized. Not both.