Why Producer-Consumer code is not working?

Question

I am trying to code solution of Producer-Consumer problem with the help of Semaphores. Below is the code I have written, on execution it is getting stuck after producing one number.

I have checked the assignment of both the Semaphore and it look fine. The sequence of acquire and release methods being called also looks correct.

public class Q {
    int n;
    Semaphore consumerSem = new Semaphore(0);
    Semaphore producerSem = new Semaphore(1);

    synchronized void get(){
        try {
            consumerSem.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Got Data : " + n);
        producerSem.release();
    }

    synchronized void put(int n){
        try {
            producerSem.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Putting Data : " + n);
        this.n=n;
        consumerSem.release();
    }
}

 public class Producer implements Runnable{
   Q q;

    Producer(Q q){
        this.q = q;
    }

    @Override
    public void run() {
        for (int i=0; i<20;i++){
            q.put(i);
        }
    }
}

public class Consumer implements Runnable{
    Q q;

    Consumer(Q q){
        this.q = q;
    }


    @Override
    public void run() {
           for (int i =0;i<20;i++){
               q.get();
           }
    }
}

public class PCRun {

    public static void main(String[] args) {
        Q q = new Q();
        new Thread(new Producer(q),"Producer").start();
        new Thread(new Consumer(q),"Consumer").start();
    }
}

Answer 1

You've made get and put synchronized. So the producer goes in, locks q, uses the one permit of producerSem and blocks at the next put call. Unfortunately, q is still locked by the producer, so the consumer won't be able to enter get. To fix this, remove both synchronizeds.

Now to synchronize the access on n, use synchronized only where n is accessed, not for both whole methods.

    int n;
    final Semaphore consumerSem = new Semaphore(0);
    final Semaphore producerSem = new Semaphore(1);

    void get() {
        try {
            consumerSem.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        synchronized (this) {
            System.out.println("Got Data : " + n);
        }
        producerSem.release();
    }

    void put(int n) {
        try {
            producerSem.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        synchronized (this) { // invariant print + write, atomic
            System.out.println("Putting Data : " + n);
            this.n = n;
        }
        consumerSem.release();
    }