Search code examples
javamultithreadingwaitrunnableillegalstateexception

Mass IllegalMonitorStateException on thread wait


When ever I call pauseThread() it always throws IllegalMonitorStateException.

I Noticed in the documentation that I need to own the Objects Monitor to be able to cause the thread to wait.

with this piece of code

synchronized (obj) {
         while (<condition does not hold>)
             obj.wait();
         ... // Perform action appropriate to condition
     }

in this case would the obj param be runner the while(ServerTickHandler.peakBlockDestructionQueue() == null){} but when obj.wait(); is called would it need to be notified? or would it notify itself when the while condition is not true would the synchronized (){} code block continually loop or would it still need a while loop within the synchronized (){} to accomplish this?

edit: would the syncronized(){} go inside by run method?

Here is my Class

public class ServerTickSaveHandler implements Runnable
{
    private static Thread   runner;
    /**
     * Creates a new thread for dealing with logging block destruction when using certain tools.
     * @param threadName Name of the thread.
     */
    public ServerTickSaveHandler(String threadName)
    {
        runner = new Thread(this, threadName);
    }
    /**
     * If thread has nothing to do we shall pause it so it does not needlessly run :D.
     * @throws InterruptedException
     */
    public void pauseThread()
    {
        try
        {
            runner.wait();
        }
        catch (InterruptedException e)
        {
            e.printStackTrace();
        }
        catch(IllegalMonitorStateException e)
        {
            e.printStackTrace();
        }
    }
    /**
     * If Items from DropItemQueue need ticking lets resume this thread.
     * @throws IllegalMonitorStateException
     */
    public void resumeThread()
    {
        try
        {
            runner.notify();
        }
        catch (IllegalMonitorStateException e)
        {
            e.printStackTrace();
        }
    }
    /**
     * The thread that is spawned when this object is created.
     */
    public void run()
    {
        while (true)
        {
            // long start = System.currentTimeMillis();

            WorldData worldData = ServerTickHandler.getBlockDestructionQueue();
            if (worldData != null)
            {
                worldData.saveToFile();
            }
            else pauseThread();

            // long end = System.currentTimeMillis();

            // NumberFormat formatter = new DecimalFormat("#0.00000");
            // Utils.log("Save Tick Handler Execution time is " +
            // formatter.format((end - start) / 1000d) + " seconds");
        }
    }
    /**
     * Starts the thread.
     * @throws IllegalStateException
     */
    public void startThread()
    {
        try
        {
            runner.start();
        }
        catch (IllegalStateException e)
        {
            e.printStackTrace();
        }   
    }
}

Solution

  • As documented, you must hold the monitor of the object on which you call wait()/notify(). Since you're calling these methods on runner, these instructions must be inside a

    synchronized(runner) {
    

    block.

    That said, calling wait()/notify() on a thread is quite a strange choice. You'd better use a final, dedicated lock Object to wait/notify. There are other bad choices in your program. For example, initializing a static field from a constructor.

    wait() and notify() are very low-level, hard to use primitives. You should use higher-level abstractions like Locks, Semaphores, CountDownLatches, BlockingQueues, etc. instead.