views:

215

answers:

5

I have a class called communicator. This class is a listener of a thread receiving events from another program. Also this class has a method call refresh that sends and action to the program a waits for the response that comes through the listener.

Both methods are in the same class but called by diferent threads.

public void processRefreshEvent(ManagerEvent event){
 //processing event
 //...
 //I'm done
 notify();
}


public synchronized void refresh() throws Exception {
 isRefreshing = true; 
 try {
                    manager.send(new refresh());
 } catch (ManagerException e) {
  isRefreshing = false;
 }

 try {
         wait(5000);
 } catch (InterruptedException e) {
 } finally{
  isRefreshing = false;
 }
}

when executing the code above I get the follow exception:

java.lang.IllegalMonitorStateException: current thread not owner
        at java.lang.Object.wait(Native Method)
        at Communicator.refresh(Communicator.java:203)
        ...

What is the proper way to "wait" for another thread to finish. Thanks.

+2  A: 

You need to synchronize your threads on a monitor. For instance (using the current object as the monitor):

public void processRefreshEvent(ManagerEvent event){
        //processing event
        //...
        //I'm done
    synchronized(this) {
        notify(); // you are basically notifying any thread who has blocked
                  // on this monitor - in our case, the instance of this object
    }
}


public synchronized void refresh() throws Exception {
        isRefreshing = true;    
        try {
                    manager.send(new refresh());
        } catch (ManagerException e) {
                isRefreshing = false;
        }

        try {
          synchronized(this) {
                wait(5000); // wait will give up the monitor
          }
        } catch (InterruptedException e) {
        } finally{
                isRefreshing = false;
        }
}
Jason Coco
Just to be clear, "any thread" means "any ONE of the waiting threads" not "all waiting threads".
erickson
Also note that the waiting thread may wake up of its own accord.
Tom Hawtin - tackline
A: 

From Object.wait()'s JavaDocs: "The current thread must own this object's monitor." So you need to synchronize on the object that you're invoking wait on.

Alternatively you could use BlockingQueue, which implements Collection and Queue. BlockingQueue does all the work of wait and notify. Your thread can just call take(), which will block until data is available. You add data to the queue using the various insert methods (add, put, etc). BTW the insert methods call notify while take() calls wait.

Steve Kuo
+1  A: 

The methods wait() and notify() may only be called from a thread that is currently synchronized on their instance.

Declare "processRefreshEvent" synchronized, or better yet, just the block of code that modifies the state that is used by the refresh method, together with the notify() call.

public void processRefreshEvent(ManagerEvent event){
  // processing event
  synchronized (this) {
    // modify shared state with results of processing.
    notify();
  }
}
erickson
A: 

Please read the JavaDoc on java.lang.Object.wait() and notify().

You should synchronize the wait() with the proper monitor, in this case:

try{
    synchronized(this){
          wait(5000);
    }
}
catch (InterruptedException e) {
} finally{
            isRefreshing = false;
}
the.duckman
+1  A: 

You say you want to wait until another thread has finished? Then just call join() on the Thread object you want to wait for.

Jorn