Multithreading - Problem with notifyAll() and wait()

Discussion in 'Java' started by Vera, Oct 13, 2006.

  1. Vera

    Vera Guest

    I have a program which has 2 threads: producer and consumer. Producer
    sleeps, wakes up and outputs how long it slept and what time it woke
    up. That info is passed to a vector. Consumer then gets that info from
    the vector and prints it out. Well, at least that's what it should do.

    Right now it gives me a runtime error because it doesn't like the
    NotifyAll() and wait() statements (producer notifies and consumer
    waits). It gives me an IllegalMonitorStateException for both - notify
    and wait, saying that "current thread is not the owner."

    Can anyone tell me what is wrong with this? I really have no idea.

    // Import libraries
    import java.util.Random;
    import java.text.DateFormat;
    import java.text.SimpleDateFormat;
    import java.text.ParseException;
    import java.util.Date;
    import java.util.Vector;

    public class Assignment3b
    {
    private Queue queue = new Queue();
    Vector vectorQueue = new Vector();
    String eventInfo;

    // Create threads:
    private ProducerThread producer = new ProducerThread();
    private ConsumerThread consumer = new ConsumerThread();


    /** Main method */
    public static void main(String[] args)
    {
    new Assignment3b();
    }

    public Assignment3b()
    {
    // Start threads
    producer.start();
    consumer.start();

    }/* End Main method */


    /** Method to get time of an event */
    public String getTimeOfEvent()
    {
    // Make a new Date object, which will be initialized to the
    current time
    Date now = new Date();

    // This will output the hour (0-12), minutes, seconds,
    milliseconds,
    // and the am/pm marker.
    SimpleDateFormat format = new SimpleDateFormat("hh:mm:ss:SS
    a");

    String outputTime = format.format(now);

    return outputTime;
    } /* End of getTimeOfEvent method */


    /***********Producer Thread *****************/
    class ProducerThread extends Thread
    {
    int sleepTime;
    String wakeTime;

    public void run()
    {
    while(true)
    {
    // Interrupt Consumer thread
    consumer.interrupt();

    // Loop Producer thread 10 times
    for (int count = 0; count < 10; count++)
    {
    // Generate random number between 10 and
    2000
    sleepTime = (int)(2000.0 * Math.random()) +
    10;

    // Put the thread to sleep for a random
    // amount of milliseconds
    try
    {
    Thread.sleep(sleepTime);
    }

    catch(InterruptedException ex)
    {
    ex.printStackTrace();
    }

    // Save the time when producer woke up
    wakeTime = getTimeOfEvent();

    // Store both times into a variable
    eventInfo = "nProducer slept for " +
    sleepTime +
    " milliseconds, and woke up at "
    + wakeTime;

    // Store the event information in the
    vector
    queue.storeEventInfo(eventInfo);

    // Wake up the consumer thread
    notifyAll();
    }
    }
    }
    }


    /*********** Consumer Thread *************/
    class ConsumerThread extends Thread
    {
    public void run()
    {
    try
    {
    // Make the thread wait while the queue is
    empty
    while(vectorQueue.isEmpty())
    {
    wait();
    }
    }

    catch(InterruptedException ex)
    {
    ex.printStackTrace();
    }

    // Output the event information produced by Producer
    thread
    while(true)
    {
    for(int count = 0; count < vectorQueue.size();
    count++)
    {

    System.out.println(vectorQueue.elementAt(count));
    }
    }
    }
    }


    /*********** Inner class for queue *************/
    class Queue
    {
    public String getEventInfo()
    {
    return eventInfo;
    }

    public synchronized void storeEventInfo(String event)
    {
    eventInfo = event;

    // Add new event to the queue
    vectorQueue.addElement(eventInfo);

    // Notify other threads
    notifyAll();
    }
    }
    }

    Please help :(
    Vera, Oct 13, 2006
    #1
    1. Advertising

  2. Vera

    hiwa Guest

    (1)wait and notify should be called in synchronized blocks or methods.
    (2)The object on which synchronization is done should be one single
    same object for the wait/notify pair. Otherwise you will get an
    IllegalMonitorStateException.
    (3)wait should be called in a while loop that tests the desired
    condition which should be changed by one of notifying threads.
    hiwa, Oct 13, 2006
    #2
    1. Advertising

  3. Vera

    Dan Andrews Guest

    Vera wrote:
    > Right now it gives me a runtime error because it doesn't like the
    > NotifyAll() and wait() statements (producer notifies and consumer
    > waits). It gives me an IllegalMonitorStateException for both - notify
    > and wait, saying that "current thread is not the owner."



    Hi Vera,

    You need to own the monitor (i.e. synchronize on something see below).
    In many cases the monitor is "this" but in your case you could use
    "Assignment3b.this" or create a new object in Assignment3b like "private
    Object lock = new Object()". You could replace lock in the code below
    with some other common object to synchronize on (i.e. replace lock with
    producer) and save the needless creation of another object called lock.
    The key here is you need a **common** object to synchronize on.
    Personally, my preference is to create the lock private variable and
    then when I see the word lock in my code it helps me with readability.


    synchronized(lock){
    while(...){
    ...
    lock.notify();
    ...
    }
    }
    .....

    synchronized(lock){
    while(...){
    ...
    lock.wait();
    ...
    }
    }

    Cheers,

    Dan Andrews
    - - - - - - - - - - - - - - - - - - - - - - - - -
    Ansir Development Limited http://www.ansir.ca
    - - - - - - - - - - - - - - - - - - - - - - - - -
    Dan Andrews, Oct 13, 2006
    #3
  4. Vera

    Dan Andrews Guest

    Hi Vera,

    I've also found you a good link that helps explain what is going on in a
    11 step summary. Good luck on your assignment!

    http://www.jguru.com/faq/view.jsp?EID=98786


    Cheers,

    Dan Andrews
    - - - - - - - - - - - - - - - - - - - - - - - - -
    Ansir Development Limited http://www.ansir.ca
    - - - - - - - - - - - - - - - - - - - - - - - - -
    Dan Andrews, Oct 13, 2006
    #4
  5. Vera

    Vera Guest

    Thank you guys, I'll check it out. I got the error to go away but I
    have another problem that's not related with the threads. But thank you
    so much for your help!! :)
    Vera, Oct 13, 2006
    #5
  6. Vera

    Guest

    In article <>,
    Vera <> wrote:
    > I have a program which has 2 threads: producer and consumer. Producer
    > sleeps, wakes up and outputs how long it slept and what time it woke
    > up. That info is passed to a vector. Consumer then gets that info from
    > the vector and prints it out. Well, at least that's what it should do.
    >
    > Right now it gives me a runtime error because it doesn't like the
    > NotifyAll() and wait() statements (producer notifies and consumer
    > waits). It gives me an IllegalMonitorStateException for both - notify
    > and wait, saying that "current thread is not the owner."



    Others have answered this question for you, but there some other
    things that seem not quite right to me:


    > Can anyone tell me what is wrong with this? I really have no idea.
    >
    > // Import libraries
    > import java.util.Random;
    > import java.text.DateFormat;
    > import java.text.SimpleDateFormat;
    > import java.text.ParseException;
    > import java.util.Date;
    > import java.util.Vector;
    >
    > public class Assignment3b
    > {
    > private Queue queue = new Queue();
    > Vector vectorQueue = new Vector();



    Why do you want both of these as instance variables? it seems a
    little odd to have your producer thread access vectorQueue indirectly
    (by way of your Queue class) while your consumer thread accesses
    it directly. Is there something I'm not getting?


    > String eventInfo;



    Why is this an instance variable? I don't quite understand, either,
    how you're using this variable -- it seems a little confused,
    and not thread-safe. Again maybe there's something I'm not getting?


    >
    > // Create threads:
    > private ProducerThread producer = new ProducerThread();
    > private ConsumerThread consumer = new ConsumerThread();



    Rather than making ProducerThread and ConsumerThread subclasses
    of Thread, you could have them implement Runnable (maybe renaming
    them Producer and Consumer) and write (untested):

    private ProducerThread producer = new Thread(new Producer());
    private ConsumerThread producer = new Thread(new Consumer());

    This might be slightly better style. It's a nitpick, maybe.


    > /** Main method */
    > public static void main(String[] args)
    > {
    > new Assignment3b();
    > }
    >
    > public Assignment3b()
    > {
    > // Start threads
    > producer.start();
    > consumer.start();
    >
    > }/* End Main method */
    >
    >
    > /** Method to get time of an event */
    > public String getTimeOfEvent()
    > {
    > // Make a new Date object, which will be initialized to the
    > current time
    > Date now = new Date();
    >
    > // This will output the hour (0-12), minutes, seconds,
    > milliseconds,
    > // and the am/pm marker.
    > SimpleDateFormat format = new SimpleDateFormat("hh:mm:ss:SS
    > a");
    >
    > String outputTime = format.format(now);
    >
    > return outputTime;
    > } /* End of getTimeOfEvent method */
    >
    >
    > /***********Producer Thread *****************/
    > class ProducerThread extends Thread
    > {
    > int sleepTime;
    > String wakeTime;
    >
    > public void run()
    > {
    > while(true)
    > {
    > // Interrupt Consumer thread
    > consumer.interrupt();



    Why are you doing this? as far as I can tell, you're ignoring
    interrupts in your consumer thread. ??


    > // Loop Producer thread 10 times
    > for (int count = 0; count < 10; count++)
    > {
    > // Generate random number between 10 and
    > 2000
    > sleepTime = (int)(2000.0 * Math.random()) +
    > 10;
    >
    > // Put the thread to sleep for a random
    > // amount of milliseconds
    > try
    > {
    > Thread.sleep(sleepTime);
    > }
    >
    > catch(InterruptedException ex)
    > {
    > ex.printStackTrace();
    > }
    >
    > // Save the time when producer woke up
    > wakeTime = getTimeOfEvent();
    >
    > // Store both times into a variable
    > eventInfo = "nProducer slept for " +
    > sleepTime +
    > " milliseconds, and woke up at "
    > + wakeTime;
    >
    > // Store the event information in the
    > vector
    > queue.storeEventInfo(eventInfo);
    >
    > // Wake up the consumer thread
    > notifyAll();
    > }
    > }
    > }
    > }
    >
    >
    > /*********** Consumer Thread *************/
    > class ConsumerThread extends Thread
    > {
    > public void run()
    > {
    > try
    > {
    > // Make the thread wait while the queue is
    > empty
    > while(vectorQueue.isEmpty())
    > {
    > wait();
    > }
    > }
    >
    > catch(InterruptedException ex)
    > {
    > ex.printStackTrace();
    > }
    >
    > // Output the event information produced by Producer
    > thread
    > while(true)
    > {
    > for(int count = 0; count < vectorQueue.size();
    > count++)
    > {
    >
    > System.out.println(vectorQueue.elementAt(count));
    > }
    > }
    > }
    > }



    This also seems a little confused .... Initially you wait until the
    first element shows up in vectorQueue, but then you start an "infinite"
    loop to repeatedly print all the elements in vectorQueue? shouldn't
    you be removing elements as you print them? and wouldn't it make more
    sense to wait repeatedly? something like this:

    while (true) {
    while (vectorQueue.isEmpty())
    wait();
    // remove element from vectorQueue and print
    }


    >
    >
    > /*********** Inner class for queue *************/
    > class Queue
    > {
    > public String getEventInfo()
    > {
    > return eventInfo;
    > }
    >
    > public synchronized void storeEventInfo(String event)
    > {
    > eventInfo = event;
    >
    > // Add new event to the queue
    > vectorQueue.addElement(eventInfo);
    >
    > // Notify other threads
    > notifyAll();
    > }
    > }
    > }
    >
    > Please help :(
    >



    --
    B. L. Massingill
    ObDisclaimer: I don't speak for my employers; they return the favor.
    , Oct 13, 2006
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. lonelyplanet999
    Replies:
    1
    Views:
    482
    A. Bolmarcich
    Nov 18, 2003
  2. Bryan
    Replies:
    12
    Views:
    685
    Thomas Hawtin
    Dec 19, 2006
  3. mallikk
    Replies:
    0
    Views:
    623
    mallikk
    Oct 24, 2007
  4. Danny
    Replies:
    18
    Views:
    803
    Daniel Pitts
    Dec 15, 2008
  5. cuffJ
    Replies:
    0
    Views:
    291
    cuffJ
    Aug 16, 2010
Loading...

Share This Page