Reputation: 3
I'm trying to create a general purpose MessagePrinter
class that handles multiple threads that each print out a message every seconds. Also, there's a manager thread that keeps track of time and wakes up each thread every second, and after each thread is woken up each thread needs to check whether it's time to print the message.
However, there's one main bug -- when I call the wait()
function when waiting to be woken up, the wait()
function isn't quitting. I've searched on Google, but to no avail. Below is the code.
import java.util.*;
class MonitorCondition {}
public class MessagePrinter implements Runnable {
private static int time = 0;
private MonitorCondition mc = new MonitorCondition();
private int elapse;
private String title;
private boolean signaled;
public MessagePrinter(int elapse, String title) {
this.elapse = elapse;
this.title = title;
new Thread(this, title).start();
this.signaled = false;
}
// http://tutorials.jenkov.com/java-concurrency/thread-signaling.html
public void run() {
if(!title.equals("manager")) {
while(true) {
synchronized(mc) {
while(!signaled) {
try { mc.wait(); }
// Now check whether we can print.
catch(InterruptedException e) {;}
}
if(time % elapse == 0)
System.out.println(elapse + " second message!");
signaled = false;
}
}
} else {
while(true) {
try { Thread.sleep(1000); } // REGARDLESS of what elapse is set to
catch(InterruptedException e) {;}
synchronized(mc) {
System.out.println(++time);
signaled = true;
mc.notifyAll();
}
}
}
}
public static void main(String[] args) {
MessagePrinter m1 = new MessagePrinter(1, "manager");
MessagePrinter m2 = new MessagePrinter(7, "client1");
MessagePrinter m3 = new MessagePrinter(5, "client2");
}
}
The code is hanging on this line: try { mc.wait(); }
. Perhaps it's a problem with how I use the synchronized blocks, but I've seen questions on producer-consumer in Java, and this looks exactly how it's supposed to be implemented according to the answers -- but this hasn't fixed the hanging wait()
function issue. Any help would be greatly appreciated.
Upvotes: 0
Views: 177
Reputation: 2803
Each instance of MessagePrinter will have its own copy of the field mc, thus each thread is entering a different monitors wait method. Calling notifyAll() does nothing because no one will ever be waiting on the managers mc monitor. Make mc static.
Edit - even though you accepted the answer already, I thought I would put this out there because your going to run into it. When you call .notifyAll() in the manager thread, it will notify all the threads waiting on that objects monitor, but only one will be able to require the lock at any given time. There is no guarantee which will acquire the lock, only that one will at any given time. That thread will release the lock then if the condition is true, it will reset signaled to false. In the mean time, one of the other threads will have woken up, and could be evaluating your while condition shortly AFTER the signaled flag was reset to false, meaning one or more of your clients may fail to print their message.
Here is an example of how it manifests in the applications output after a while of running:
7 second message!
561
562
563
564
565
5 second message!
566
567
568
569
570
571
572
573
574
7 second message!
575
5 second message!
Upvotes: 1