IllegalMonitorStateException while calling notifyAll

Question

I have a piece of code which is giving IllegalMonitorException while calling notifyAll. While enQueue(), thread add the data and throws illegalMonitorException.

BlockingQueue.class :

public class BlockingQueue {
private Queue<Integer> queue = new LinkedList<>();
private int maxCount;
private Lock lock = new ReentrantLock();

BlockingQueue(int maxCount){
    this.maxCount=maxCount;
}

public  void enQueue(int d) throws InterruptedException {
    lock.lock();
    if(queue.size() >= maxCount){
        System.out.println("going to wait enqueu "+Thread.currentThread().getName() +" size "+queue.size());
        lock.wait();
    }
    System.out.println(" Adding "+d);
    queue.add(d);
    lock.notify();
    lock.unlock();
}

public  Integer deQueue() throws InterruptedException {
    lock.lock();
    if(queue.size()==0){
        System.out.println("going to wait dequeue "+Thread.currentThread().getName()+" size "+queue.size());
        lock.wait();
    }
    int data = queue.remove();
    lock.notify();
    lock.unlock();
    return data;
}}

Main.Class :

 public class Main {
public static void main(String args[]){
    BlockingQueue queue=new BlockingQueue(10);
    Producer p = new Producer(queue);
    Consumer c = new Consumer(queue);

    Thread t1=new Thread(c,"Consumer");
    Thread t2=new Thread(p, "producer");

    t1.start();
    t2.start();
}}

Producer.class :

 public class Producer implements Runnable {
private BlockingQueue q;
Producer(BlockingQueue qu){
    this.q=qu;
}

public Integer generateWork() throws InterruptedException {
    return new Random().nextInt(100 );
}

@Override
public void run() {
    for(int i =0; i<100; i++){
        try {
            Thread.sleep(100);
            q.enQueue(generateWork());
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}}

Consumer.class :

public class Consumer implements Runnable {

private BlockingQueue queue;
Consumer(BlockingQueue q){
    this.queue=q;
}

public void consume(int data){
    System.out.println(" consume "+data);
}

@Override
public void run() {
    for(int i=0; i < 100; i++){
        try {
            Thread.sleep(1000);
            consume(queue.deQueue());
        } catch (InterruptedException e) {
           System.out.println("interrupted");
        }
    }
}}

Output :

Adding 94

Exception in thread "producer" java.lang.IllegalMonitorStateException

at java.lang.Object.notify(Native Method)
at BlockingQueue.enQueue(BlockingQueue.java:23)
at Producer.run(Producer.java:20)
at java.lang.Thread.run(Thread.java:748)

Could you post full code that can be ran to reproduce the issue? — Yogesh Badke, Jan 25 '20 at 06:45
Hey.. i have updated the code. I dont want to use condition. :/ — Rohit Rawat, Jan 25 '20 at 06:56
If you're using `Lock` then you **have to** use `Condition`. But why are you hesitant to use `Condition`? — Slaw, Jan 25 '20 at 07:02
@Slaw it is not compulsory to use `Condition` with `Lock` but yeah its a great combo. — Yogesh Badke, Jan 25 '20 at 07:08
@YogeshBadke It is if you want to use the API correctly. While your answer may work, one should _either_ use `synchronized` with `Object`'s wait/notify _or_ `Lock` with `Condition`'s await/signal. Using `synchronized (lock)` where `lock` is an instance of `java.util.concurrent.locks.Lock` is a code smell. — Slaw, Jan 25 '20 at 07:10
@YogeshBadke The reason `Condition` exists is for people not writing code such the OP did. — , Jan 25 '20 at 07:14
100% agree on what you are all saying! I am not denying. I am just saying this is what OP asked for **explicitly**. — Yogesh Badke, Jan 25 '20 at 07:15

Yogesh Badke · Answer 1 · 2020-01-25T07:27:12.657

Following code fixes the issue

public class BlockingQueue {
    private Queue<Integer> queue = new LinkedList<>();
    private int maxCount;
    private Lock lock = new ReentrantLock();

    BlockingQueue(int maxCount) {
        this.maxCount = maxCount;
    }

    public void enQueue(int d) throws InterruptedException {
        lock.lock();
        if (queue.size() >= maxCount) {
            System.out.println("going to wait enqueu " + Thread.currentThread().getName() + " size " + queue.size());
            waitInternal();
        }
        System.out.println(" Adding " + d);
        queue.add(d);
        notifyInternal();
        lock.unlock();
    }

    public Integer deQueue() throws InterruptedException {
        lock.lock();
        if (queue.size() == 0) {
            System.out.println("going to wait dequeue " + Thread.currentThread().getName() + " size " + queue.size());
            waitInternal();
        }
        int data = queue.remove();
        notifyInternal();
        lock.unlock();
        return data;
    }

    private void waitInternal() throws InterruptedException {
        synchronized (lock) {
            lock.wait();
        }
    }

    private void notifyInternal() throws InterruptedException {
        synchronized (lock) {
            lock.notify();
        }
    }
}

Explanation: You were calling wait and notify from the non-synchronized block which is not allowed. When you call either of these, you should take a monitor lock on that object. In this case lock.

Here is a great read on why must wait/notify always be in a synchronized block https://stackoverflow.com/a/2779565/1891456

IllegalMonitorStateException while calling notifyAll

1 Answers1