Re: Multithreading - Problem with notifyAll() and wait()
In article <1160709488.295060.90110@m7g2000cwm.googlegroups.com>,
Vera <vera13@gmail.com> 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.