Producer Consumer using Reentrant lock not working

Question

I am trying to implement consumer-producer using a ReentrantLock as given below:

class Producer implements Runnable {

    private List<String> data;
    private ReentrantLock lock;

    Producer(List<String> data,ReentrantLock lock)
    {
        this.data = data;
        this.lock = lock;
    }

    @Override
    public void run() {
        int counter = 0;
        synchronized (lock)
        {
            while (true)
            {

                if ( data.size() < 5)
                {
                    counter++;
                    data.add("writing:: "+counter);
                }
                else
                {
                    try {
                        lock.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        }

    }
}

class Consumer implements Runnable{

    private List<String> data;
    private ReentrantLock lock;

    Consumer(List<String> data,ReentrantLock lock)
    {
        this.data = data;
        this.lock = lock;
    }

    @Override
    public void run() {
        int counter = 0;
        synchronized (lock)
        {
            while (true)
            {

                if ( data.size() > 0)
                {
                    System.out.println("reading:: "+data.get(data.size()-1));
                    data.remove(data.size()-1);
                }
                else
                {
                    System.out.println("Notifying..");
                    lock.notify();
                }
            }
        }

    }
}

public class ProducerConsumer  {

    public static void main(String[] args) {
        List<String> str = new LinkedList<>();
        ReentrantLock lock= new ReentrantLock();
        Thread t1 = new Thread(new Producer(str,lock));
        Thread t2 = new Thread(new Consumer(str,lock));
        t1.start();
        t2.start();
    }
}

So, it writes only once to the list and then Consumer waits indefinitely. Why is this happening? Why not Producer is acquiring the lock?

Answer 1

I want to point out two mistakes you have made:

1. The wrong usage of ReentrantLock.

Every object can be used for an intrinsic lock, so there is no need to find a specific Lock class. Since each synchronized block is bounded by a single method and you don't demand any enhanced means, ReentrantLock is redundant here.

2. The incorrect position of the synchronized blocks.

Once you enter a synchronized block, no one can enter there until you leave it. Obviously, you will never exit it because of while(true).

I would suggest you remove the ReentrantLocks and do synchronisation on data.

@Override
public void run() {
    int counter = 0;
    while (true) {
        synchronized (data) {
            if (data.size() < 5) {
                data.add("writing:: " + ++counter);
            } else {
                try {
                    data.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
        // we are out of the synchornized block here
        // to let others use data as a monitor somewhere else
    }
}