0

To illustrate my problem, and as we are in easter eggs period, here is the storyboard : The first character is a clock is giving periodically the time. But this clock is very moody : it doesn't answer to a user who ask time but inform all his observers periodically and this period is randomly defined. My second character is a very stressed rabbit. This rabbit can't do anything unless he knows the time. And when he finished his action, he asks again the time and waits to get it before doing something else. I could add other characters (a cat, a mad hat-maker...) but for this example, it's not necessary.

So, in Java, I'll have a clock which is observable by observers, whatever the type of observers ; and a rabbit which is a type of observer. I want to keep the pattern «observable / observer» as the clock don't know and don't care who are the observers.

Here are the class I'm using :

Clock

public class Clock implements Runnable, ClockObservable {
    private Long time;

    public Clock () {

    }

    @Override
    public void run() {
        while (true) {
            time=System.currentTimeMillis();
            update();
            try {
                int randomTimeUpdate=(int)( (Math.random() + 1) *500); //This clock will update randomly
                Thread.sleep(randomTimeUpdate);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }   
    }

    @Override
    public void update() {
        for (ClockObserver observer : observers) {
            observer.update(time);
        }       
    }
}

ClockObservable

import java.util.ArrayList;

public interface ClockObservable {
public static ArrayList<ClockObserver> observers = new ArrayList<ClockObserver>();

    public default void addObserver (ClockObserver observer) {
        observers.add(observer);
    }

    public default void resetObservers () {
        observers.clear();
    }

    public void update ();

}

ClockObserver

public interface ClockObserver {

    public void update(Long time);

}

Rabbit

public class Rabbit implements ClockObserver {

    Long time;

    public Rabbit (Clock clock) {
        clock.addObserver(this);
        whatTimeIsIt();
    }

    public synchronized void whatTimeIsIt () {
        while (true) {
            System.out.println("What time is it ?");
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            System.out.println("It's "+time+" -> Yepeeeee !!!! I can do something before asking the time again...");
            System.out.println("-----------------------------------");
        }
    }

    @Override
    public void update(Long time) {
        this.time=time;
        System.out.println("Time = "+time);
        //How can I notify the rabbit ?
        Rabbit.this.notifyAll(); //This cause a «java.lang.IllegalMonitorStateException»
    }
}

Main

public class Main {

    public static void main(String[] args) {
        Clock clock = new Clock();
        Thread thread = new Thread (clock);
        thread.start();
        new Rabbit(clock);
    }
}

The problem is the following instruction in the Rabbit class, in the overrided update method, which generates a «java.lang.IllegalMonitorStateException».

Rabbit.this.notifyAll();

And, indeed, the Rabbit is on the Main thread and the notify is on the thread-0.

But, what can I do ? How to solve my problem ?

Thank you for all your answers.

Dr_Click

Dr_Click
  • 449
  • 4
  • 16
  • Why are you calling `notifyAll`? No state has changed, so there is nothing to notify other threads about. How you fix the code depends on what the state is that was being waited for and that you are notifying about. You `whatTimeIsIt` thread just calls `wait` without actually waiting for anything. There has to be something that it is waiting for and that something has to be shared state that is synchronized between threads. You cannot use notify/wait any other way, – David Schwartz Apr 15 '20 at 08:25
  • I'd like this behavior : the rabbit in a waititng state and does nothing. When the clock send the time, the rabbit receive the time so the value of «time» is updated. This update allow the rabbit to do something. And when this something else is finished, the rabbit waits a new update to do something (the same thing or something else, I'll define this point later) – Dr_Click Apr 15 '20 at 08:29
  • The rabbit isn't waiting *for* anything. If it was, there would be some check to see if that thing had already happened, which there isn't. Imagine if the thread gets delayed by the scheduler before it calls `wait` and then the thing it's waiting for happens, it will still dumbly call `wait`. Your code isn't at all right. You don't understand what `wait`/`notify` is *for* and why you need an atomic "unlock and wait" operation. [This answer](https://stackoverflow.com/a/56384884/721269) may be helpful. – David Schwartz Apr 15 '20 at 08:30

1 Answers1

1
public void update(Long time) {
    this.time=time;
    System.out.println("Time = "+time);
    //How can I notify the rabbit ?
    Rabbit.this.notifyAll(); //This cause a «java.lang.IllegalMonitorStateException»
}

The notifyAll function is used to notify other threads that some shared state has changed. No shared state has changed here, so there is nothing to notify other threads about. If you're thinking this.time is shared state, then explain why an unsynchronized method is modifying it without holding any lock. You can't do that with shared state.

public synchronized void whatTimeIsIt () {
    while (true) {
        System.out.println("What time is it ?");
        try {
            wait();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

Same problem here. You call wait without checking that the thing you are waiting for hasn't already happened. What are you waiting for? What is the shared state that is not in the state you need it to be in?

Imagine if one thread is about to call wait but the scheduler delays it. Then, the other thread calls notifyAll. Your thread would still call wait because it didn't check the shared state to see if it was supposed to wait.

You cannot use wait except to wait for some piece of shared state to have some value. You cannot use notify except to notify another thread that the shared state has changed.

Your current code just doesn't make any sense because it isn't waiting for anything nor is it notifying about anything. The notify/wait functions do not have the semantics of sempahores. They do not have their own shared state. You have to implement the shared state.

If the rabbit is in a waiting state, somewhere you need some variable that holds the rabbit's state and it needs to be set to "waiting". Then, when you want to change the rabbit's state, you need to change that state to "running". Then the rabbit can wait for its state to set to "running" and the other thread can notify it that its state had changed. But you didn't implement any shared state, so there is nothing to wait for and nothing to notify about. And, of course, that state variable needs to be protected by a lock.

The rabbit should have code like while (state == waiting) wait(); and the other thread can have code like state = running; notifyAll();. Then the rabbit is actually waiting for something and the other thread has modified some shared state that it may need to notify the other thread about. Of course the state should only be changed or tested while holding the lock.

Also, why isn't update a synchronized method? It changes time which is shared.

Here's another option:

public synchronized void whatTimeIsIt () {
    Long last_time = time; // make local copy
    while (true) {
        System.out.println("What time is it ?");
        try {
            while (time == last_time)
                wait();
            last_time = time;
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("It's "+time+" -> Yepeeeee !!!! I can do something before asking the time again...");
        System.out.println("-----------------------------------");
    }
}

Notice how the thread is now waiting for something? And notice how that something is shared state between the two threads?

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • 1
    Thank you very much. Your explanation is clear and I'll dig the topic to learn more about it. My update method also had to be synchronized to make it working like you wrote in your post. – Dr_Click Apr 15 '20 at 12:29