Why does thread wait(milis) halt scheduled executor?

Question

I'm trying to wrap my head around threads and I've thought of this particular case: A scheduled runnable which will poll data from somewhere and when there is no data to poll the thread should wait for a while and then start polling again.

This is an example code of what I was trying to accomplish

public class ThreadTest {
    public static void main(String[] args) {
        Object lock = new Object();
        ScheduledExecutorService scheduledExecutorService = Executors.newScheduledThreadPool(10);
        ScheduledFuture<?> scheduledFuture = scheduledExecutorService.scheduleWithFixedDelay(new Worker(lock), 100, 1000, TimeUnit.MILLISECONDS);

        while(!scheduledFuture.isDone()) {
        }

        System.out.println("Main finished");
    }

    public static class Worker implements Runnable {

        private final Object lock;
        public Worker(Object lock) {
            this.lock = lock;
        }

        @Override
        public void run() {
            long startMilis = 0;
            try {
                synchronized (lock) {
                    System.out.println(Thread.currentThread().getName() + " Simulating work by sleeping for 3 seconds");
                    Thread.sleep(3000);
                    startMilis = System.currentTimeMillis();
                    System.out.println(Thread.currentThread().getName() + " Work done. Initiating wait for 200ms");
                    // this is supposed to pause the thread and release the lock and then wake up the thread after 200ms
                    // however what it seems to do is it puts the thread in the waiting state but it does not wake up
                    // after 200ms and resume, instead it skips to the finally block and completes the scheduled future
                    // stopping any further threads to start... why?
                    wait(200);
                    // this pauses the thread, keeping the lock, and the thread finishes after sleep. Works as intended.
                    //Thread.sleep(200); 
                }
                System.out.println(Thread.currentThread().getName() + " Done waiting. Exiting");
            } catch (InterruptedException e) {
                throw new RuntimeException(e);
            } finally {
                long endMilis = System.currentTimeMillis();
                long difference = endMilis - startMilis;
                System.out.println(Thread.currentThread().getName() + " Finally block reached after " + difference + "ms");
            }
        }
    }
}

If you run this code as-is it will spawn a thread running the Worker runnable, the runnable will sleep for 3 seconds simulating some long lasting work, then it will hit the wait(200) call and the thread will immediately go to the finally block and exit and the ScheduledFuture will complete. Why is that? I thought that wait(milis) could be used for low-power polling, having the thread release it's lock and wait for some time before running again but this is only causing me more confusion. Can anyone elaborate?

Answer 1

It's because you are calling Object.wait(), and that throws

IllegalMonitorStateException - if the current thread is not the owner of the object's monitor

your thread does not own the monitor for this (the worker), so it throws.

As you only catch InterruptedException, it is not caught within the runnable (but the finally block executes).

ScheduledThreadPoolExecutor.scheduleWithFixedDelay() documents:

The sequence of task executions continues indefinitely until one of the following exceptional completions occur:

[...]
An execution of the task throws an exception. [...]

therefore it terminates.

---

As the answer by rzwitserloot already provides a great explanation about how Object.wait() and Object.notify() are meant to be used, let me just add that there is an alternative implementation of the underlying concept (called Condition Variable) in the class Condition which includes a code example for a use case in the JavaDocs.

Answer 2

You're not using wait correctly.

Here's a very short manual to wait/notify (these 2 methods go hand in hand, there is zero point using one of them without also using the other!)

We have 2 threads, we shall name them Alice and Bob. Alice wants to wait, but, instead of the 'condition for when you can cease waiting' being: "After X milliseconds have passed", the condition we want to wait for is "When event Z happens, and Z is something another thread (Bob, in our case), will witness and will want to tell us about".

To get the job done, first, we need Alice and Bob to both have a reference to the same object. Generally, because threading and locking is quite tricky, this should be non-global - i.e. something alice and bob have, but the rest of the wider code base doesn't. Failure to adhere to this rule makes it almost impossible to reason about code because you'll have no idea who is capable of messing with that 'lock' object. For the same reason you should replace public fields with private ones and if needed access methods (setters and getters), do not have public locks. Unfortunately, synchronized defaults to this if you stick it on a method, so, never do that unless you really, really know what you are doing. Instead, make private lock objects.

Then, here's the flow:

1. Alice LOCKS on the lock. (`synchronized (lock) { ... }).
2. Whilst holding it (inside the braces that go with the block), alice runs into the situation where alice wants to wait until event Z happens. She calls lock.wait() on the lock. This will both freeze the Alice thread as well as release the lock.
3. Bob locks on the exact same lock object. This will cause bob to wait (he didn't call lock.wait, he tried to entered a synchronized block which can also cause waiting to occur) until Alice either exits her synchronized block or invokes wait on the lock.
4. Bob does some stuff and eventually wants to wake Alice up, so bob calls lock.notifyAll() (I generally advise not calling notify - the difference is mostly esoteric and usually irrelevant, though). Bob can only do this when he holds the lock (inside a synchronized (lock) {} block).
5. This does not actually wake Alice up! - after all, for alice to wake up, alice has to re-acquire the lock (that lock.wait() call is inside synchronized (lock) {} after all, and bob holds that lock right now; has to be, because Bob can't call lock.notifyAll() unless bob holds that lock!). It merely sets alice up to wake when bob's done with their lock.

And.. that's all there is to it.

This isn't necessarily the best way to do locking; often something from the java.util.concurrent package is a better idea.

The problem in your code is that you just call wait(), which therefore resolves to this.wait(), but you wanted lock.wait().