Producer / Consumer problem using threads

Sorry for the long post, but I'm just getting started with Java.
I'm trying to implement a simple Producer / Consumer model, where one thread puts things on a stack, and another takes them off. In this case the Stack is populated by some packets coming from a UDP port, but that's not the problem here.
In the following code, the consumer *the pop()* never gets called. I'm sure there is a simple reason why... Any help _much_ appreciated....

import UDPStack;
import java.util.*;

public class Main extends Thread {


    public class Producer extends Thread {

      UDPStack stack;

      public Producer( UDPStack s) {
          stack = s;
      }

      public void run() {
          synchronized(stack) {
            stack.listen();
            stack.notify();
            try {
                stack.wait();
            }
            catch (Exception e) {};
          }
          try {
            this.sleep(1000);
          }
          catch (Exception e) {};
          Thread.yield();
      }
    }
      
    public class Consumer extends Thread {

      UDPStack stack;
      
      public Consumer ( UDPStack s) {
          stack = s;
      }
      
      public void run() {
          while (true) {
            try {
               
                System.out.println("Waiting for lock on object()");
                synchronized(stack) {                     
                  stack.wait();
                }
            }
            catch (InterruptedException e) {};
            
            System.out.println ( "Popped: " + stack.pop());
            
          }
      }
    }

    public static void main(String[] args) {
      Main m = new Main();
    }

    public Main() {
      Stack s = new Stack();      
      UDPStack u = new UDPStack ( 34567, s );
      u.setup();
      
      Producer p = new Producer(u);
      Consumer c = new Consumer(u);
      p.start();
      c.start();
      try {
          Thread.currentThread().sleep(5*1000);
      }
      catch (InterruptedException e) {};
    }
      

}

I'm sure that the question is not hard, but id appreciate a fast answer. What am I doing wrong? TIA!
time4teaAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Ravindra76Commented:
Hi ,

One way of solution is making variable to static and methods synchronized

Best of luck
0
nil_dibCommented:
change

p.start();
c.start();

to

c.start();
p.start();

nil_dib
0
time4teaAuthor Commented:
No, this does not work, and even if it did, would be incorrect. There should be no difference which one gets started first.

Perhaps I should say that the Producer contains a class that blocks until a new packet arrives, so nearly all the time is blocked in the call stack.listen().

I understand that the consumer should be notifyed when the producer has added something to the queue, but dont _how_ to accomplish this...

time4tea
0
Exploring ASP.NET Core: Fundamentals

Learn to build web apps and services, IoT apps, and mobile backends by covering the fundamentals of ASP.NET Core and  exploring the core foundations for app libraries.

JodCommented:
Trying to understand your code here, so...

Presumably this run method in the Producer thread will listen for and place at least one item on the stack?

  public void run() {
    synchronized(stack) {
      stack.listen();
      stack.notify();
      try {
        stack.wait();
      } catch (Exception e) {};
    }

    try {
      this.sleep(1000);
    } catch (Exception e) {};
      Thread.yield();
    }
  }

but only runs once. And then this run method in the consumer thread waits to be signalled (by stack.notify()?)

public void run() {
  while (true) {
    try {

      System.out.println("Waiting for lock on object()");
     
      synchronized(stack) {      
        stack.wait();
      }
    }

    catch (InterruptedException e) {};

    System.out.println ( "Popped: " + stack.pop());

    }
  }
}

At first sight, it would seem that the consumer is waiting on the stack in stack.wait() but is synchronized. The problem is, the producer is also waiting on stack.wait() but has already locked the stack before the consumer can get to it.

eg as above:

Producer:

    synchronized(stack) {
      stack.listen();
      stack.notify();
      try {
        stack.wait();
      } catch (Exception e) {};
    }

consumer:

      synchronized(stack) {      
        stack.wait();
      }

The producer is probably locking the consumer out as the producer starts first - the consumer therefore never actually gets to the stack. Or if it does, how does the consumer get notified to wake up from it's stack.wait state if the producer has already been woken up from it's stack.wait state. I need to know more about how your code works here.

Try this in your consumer thread:

public void run() {
  while (true) {

    try {
      System.out.println("Waiting for lock on object()");

      synchronized(stack) {      
        stack.wait();
      }

      System.out.println ( "Popped: " + stack.pop());
    }

  }
}



But your producer needs to notify the consumer so the consumer should wake up when the producer does notify (that's what this is for yeah?). What you need is for stack.wait() to release the consumer when an event has occurred - I don't think your producer needs to be waiting on stack.wait(), as it has already placed an event on the queue using listen and notify.

So I would change the producer to do this:

  public void run() {

    // put something on stack then unlock it...
    synchronized(stack) {
      stack.listen();
      stack.notify();
    }

    // do you need to do this....?
    try {
      this.sleep(1000);
    } catch (Exception e) {};
      Thread.yield();
    }
  }

You may need to clarify my understanding of how your UDPstack class works if this is not the way you intended it.

ALSO, when the producer does stack.listen() does it ever get woken from this state to continue with the code? try a println statement after to see where it gets to.

To be honest, the way I see it is both your producer and consumer are listening. In general this won't work...

The producer should work on a fire and forget method - it locks the stack, puts an event on the stack and then moves on to wait for the next event. The consumer is in always in a sleep state that it ONLY gets woken from when the producer puts something on the stack. It then processes this item and then goes back to waiting again.

Makes sense?

So what is actually putting things on the stack? If it is not the producer, where does this get done?

Because whatever isputting things on the stack IS your actual producer - the producer above is just a red herring.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
mbormannCommented:
example

             import java.io.*;
             /**
             * This class is the Starter class for the 2 threads which will act as
             Classical Thrower and Catcher
             */

             public class PipedIOApp extends java.lang.Object
             {
             public static void main( String [] args )
             {
             Thread producerThread = new Thread ( new PipeOutput("Thrower")
             );
             Thread consumerThread = new Thread ( new PipeInput("Catcher")
             );

             producerThread.start();
             consumerThread.start();

             boolean isproducerThreadAlive = true;
             boolean isconsumerThreadAlive = true;

             do
             {
             if( isproducerThreadAlive && !producerThread.isAlive() )
             {
             isproducerThreadAlive = false;
             System.out.println("ProducerThread is dead");
             }
             if( isconsumerThreadAlive && !consumerThread.isAlive() )
             {
             isconsumerThreadAlive = false;
             System.out.println("ConsumerThread is dead");
             }
             }
             while( isproducerThreadAlive  || isconsumerThreadAlive );
             }
             }
             /**
             * This class will connect the Threads to the PipedOutputStream &
             PipedInputStream
             * for inter Thread communication
             */

             class PipeIO
             {
             String nameID = null;

             public PipeIO( String passedID )
             {
             nameID = passedID;
             }

             static PipedOutputStream outputPipe = new PipedOutputStream();
             static PipedInputStream inputPipe = new PipedInputStream();
             static
             {
             try
             {
             //Connect the Input & Output Pipes together
             outputPipe.connect( inputPipe );
             }
             catch( Exception e )
             {
             System.out.println( "Exception in static initializer in class PipeIO "
             + e.toString() );
             }
             }
             }


             /**
             */

             class PipeOutput  extends PipeIO implements Runnable
             {
             public PipeOutput( String passedID )
             {
             super( passedID );
             }

             public void run()
             {
             String temp = "Strings This is a test run Only";
             try
             {
             for( int i= 0; i < temp.length();i++ )
             {
             outputPipe.write( temp.charAt(i) );
             System.out.println( nameID + " wrote " + temp.charAt(i) );
             }
             outputPipe.write('!');
             }
             catch( Exception e )
             {
             System.out.println( "Exception in PipeOutput" + e );
             }
             }
             }

             /**
             */

             class PipeInput extends PipeIO implements Runnable
             {
             public PipeInput( String passedID )
             {
             super( passedID );
             }

             public void run()
             {
             boolean EOF = false;

             try
             {
             while( !EOF )
             {
             int inChar = inputPipe.read();
             if( inChar != -1 )
             {
             char oneChar = (char)inChar;
             if( oneChar == '!')
             {
             EOF = true;
             break;
             }
             else
             System.out.println( nameID + " reads " + oneChar );
             }
             }
             }
             catch( Exception e )
             {
             System.out.println( "Exception in PipeInput" + e );
             }
             }
             }
0
time4teaAuthor Commented:
You star!
OK, the problem was (i chagned the code to loop as you suggested), but found that the loop was not looping.

What was happening was that the stack.listen() function looped forever, and thus the synchronized block was never exited. Thus the consumer bit never got the lock.

Thanks for your setting on the correct path!

time4tea
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java

From novice to tech pro — start learning today.