In Producer consumer problem inter thread communication is not happening the code gets stuck at a ce-CodePudding

I was doing the producer consumer problem in java but my code is getting stuck at System.out.println("In condition in produce method " ) this line.I think due to which my code is not abele to execute the consume method. So can anyone help me with this and tell me what is the reason behind this and what modifications should be done in the code.


import java.util.LinkedList;
class Consumer extends Thread {
    public void run()
    {
        try{
            PC p = PC.getInstance();

            p.consume();
            Thread.sleep(500);
        }
        catch(Exception e ){
        System.out.println(e);
    }
    }
}
class Producer extends Thread{

    public void run (){
        PC p = PC.getInstance();
        try{

            p.produce();
            Thread.sleep(500);
        }
        catch(Exception e){
            System.out.println("interrupted");
        }
    }
}
class PC {
     LinkedList a = new LinkedList();
    int capacity = 2;
    private static PC single_instance = null;

    public static PC getInstance()
    {
        if (single_instance == null)
            single_instance = new PC();

        return single_instance;
    }

    public    void consume() throws InterruptedException{
        while (true) {
            synchronized (this) {
                while (a.size() == 0) {
                    System.out.println("here I am  ");
                    wait();
                }

                int val = (int) a.removeFirst();
                System.out.println("consumer consumed"   val);
                notify();
                Thread.sleep(500);
            }
        }
    }
    public    void produce() throws InterruptedException {

       while(true) {
           int value = 0;
           synchronized (this) {

               while (a.size() == capacity) {
                   System.out.println("In condition in produce method " );////
                   wait();
               }


               System.out.println("producing  value "   value);
               a.add(value  );
               notify();
               System.out.println("after notify in produce method" );
               Thread.sleep(500);
           }
       }
    }
}
public class multithreading {
    public static void main(String[] args) throws InterruptedException {

        Producer p1 = new Producer();
       // Producer p2 = new Producer();

        Consumer c1 = new Consumer();
        p1.start();
        c1.start();

        p1.join();
        c1.join();
    }
}

I am getting output till this only

producing  value 0
after notify in produce method
producing  value 0
after notify in produce method
In condition in produce method 

Process finished with exit code 130

CodePudding user response：

This is bad:

synchronized(this) {
    ...
    Thread.sleep(...);
    ...
}

It's bad because the thread that's sleeping has locked other threads out of the critical section for no good reason. It's like, two housemates share a car, but one of the housemates takes the keys and then goes to bed. Why should the other housemate not be allowed to use the car when the first housemate is sleeping?

This also is bad:

while (true) {
    synchronized(this) {
        ...
    }
}

It's bad because the very next thing that the thread does after leaving the critical section is, it tries to re-enter the critical section. Java's intrinsic locks are not fair. The thread that just left the synchronized blocks is already running when it tries to re-enter. The other thread is blocked waiting its turn. In that situation, the OS is always going to choose the thread that already is running, because in well-architected program—I.e., in a program where one housemate doesn't go to bed with the car keys—that's the strategy that usually will yield the best performance.

Move the System.out.println() calls and the Thread.sleep() calls out of the synchronized(this) block, and that will give the other thread a better chance to run:

while (true) {
    synchronized(this) {
        ...
        notify();
    }
    System.out.println("...");
    Thread.sleep(500);
}

@Gardener may already have identified your problem: Your singleton is not thread-safe (see Gardener's comment, above). You should synchronize the static getInstance() method on some static object. (the PC class object would work.)

public static PC getInstance()
{
    synchronized(PC.class) {
        if (single_instance == null) {
            single_instance = new PC();
        }

        return single_instance;
    }
}

This is not great:

...
catch (Exception e) {
    System.out.println(e);              // not great
    System.out.println("interrupted");  // even less great
}

It's not great because it gives you only minimal information if an exception occurs. Do this instead to get a detailed message on the standard error output stream that tells exactly where the exception happened:

...
catch (Exception e) {
    e.printStackTrace();
}