IllegalMonitorStateException when attempting to wait() a thread

Question

I have done some research to my problem and am still unable to fix the issue at hand. Threading is new to me so Im having trouble comprehending. In my program, I am starting a thread that is transferring files on a timed period. SWT is being used for the GUI in this program. In my main UI code I have a pause and play button. The play button and related code:

    playButton.addSelectionListener(new SelectionAdapter() {
        @Override
        public void widgetSelected(SelectionEvent e) {

            if(isRunning){
                // TODO implies runningThread is waiting, notify it
            }else{
                playButton.setEnabled(false);
                pauseButton.setEnabled(true);
                try {
                    play();
                } catch (IOException e1) {
                    e1.printStackTrace();
                }
            }
        }
    });

public void play() throws IOException{

    if(controller.timMan.getEventSendPreferences().equalsIgnoreCase("manual")){
        isRunning = true;
        manualThread.start();


    }else if(controller.timMan.getEventSendPreferences().equalsIgnoreCase("timed")){
        isRunning = true;
        timerThread.start();
    }

    return;
}

timerThread is implemented as such:

timerThread = new Thread(new RunOnTimer(controller));

public static class RunOnTimer implements Runnable{

    ScriptController controller;

    public RunOnTimer(ScriptController c){
        controller = c;
    };

    @Override
    public void run(){
        try{
            synchronized(this){
                controller.runOnTimer();
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
};

and here is runOnTimer() function called in run:

public void runOnTimer() throws IOException{

    for(File f : dirMan.getEventFileList()){
        int randomTimeValue = 0;
            int range = timMan.getUpperTimerBound() - timMan.getLowerTimerBound();
            if(range > 0)
                randomTimeValue = (new Random().nextInt(timMan.getUpperTimerBound() - timMan.getLowerTimerBound()) + 0);

            try {
                Thread.sleep(1000 * (randomTimeValue + timMan.getLowerTimerBound()));
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            dirMan.updateFileCounts();
            System.out.println("Event " + dirMan.getSentEventFiles());
            File dest = new File(dirMan.getDestinationFolder() + "\\" + f.getName());
            Files.copy(f.toPath(), dest.toPath(), StandardCopyOption.REPLACE_EXISTING);
    }   
    updateFileCounts();
    return;
}

The problem starts not while the thread is running but when I call the thread to wait in the paused buttons listener implementation.

pauseButton.addSelectionListener(new SelectionAdapter() {

        @Override
        public void widgetSelected(SelectionEvent e) {
            if(timerThread.isAlive()){
                try {
                    timerThread.wait();
                } catch (InterruptedException e1) {
                    // TODO Auto-generated catch block
                    e1.printStackTrace();
                }
            }

            pauseButton.setEnabled(false);
            playButton.setEnabled(true);
        }

    });

In the other questions I have read and from what google has results have showed, synchronizing is usually the problem when it comes to this error. Something to do with getting the owner of the objects monitor. Again I have not worked with threading so this concept of object monitors is a mystery to me. So after reading in on these I tried using synchronized in the run() method of the RunOnTimer class but didn't seem to change anything when attempting to 'pause' (ie. make the thread wait).

What is it I am missing or doing wrong. The program will work fine and the thread will run like I expected it to except for the error of course.

Answer 1

If you read documentation for Object.wait you will see this crucial piece of information:

The current thread must own this object's monitor.

What it means is that you have to have a synchronized block on the object for which you call wait. The documentation, actually, has a nice pseudo code that shows correct usage of wait

In your case:

synchronized(timerThread)
{
  while( shouldStillWait() )
  {
    timerThread.wait();
  }
}

boolean shouldStillWait( )
{
   ...
}

Also there should be a place somewhere in your code that calls timerThread.notify(), without it you will wait forever.

That said, since Java 5, there are much better built-in synchronization primitives that make use of wait/notify pretty much thing of the past.

And this is a GOOD THING, because wait/notify is very brittle and error prone.

I suggest reading excellent 'Java Concurrency in Practice' by Brian Goetz, to get familiar with the new synchronization primitives and good style multi-threaded programming.

Answer 2

The problem is that you cannot call timerThread.wait() in the listener of the pause button because that entire method body runs in the event handling thread of the application and thus the method does not own the object monitor of timerThread.

So, in order, to achieve your desired functionality you should have a variable to maintain the state of the application (i.e. whether it is paused or not).

In your main application class you can declare a boolean variable

private boolean paused;

Inside your pause button event handling method you should remove

        if(timerThread.isAlive()){
            try {
                timerThread.wait();
            } catch (InterruptedException e1) {
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }
        }

and replace it with

paused = !paused; // to just toggle the state
timerThread.notifyAll(); // to notify waiting thread

Then in your Runnable it will be

@Override
public void run(){
    try{
        while (paused) {
            synchronized(this) {
                wait(); // keep sleeping this thread while the state is still paused
            }
        }
        controller.runOnTimer();
    } catch (IOException e) {
        e.printStackTrace();
    }
}