Currently I'm working on multi-thread window's interface and I can't understand why the following simplified code doesn't work:

Window.java

public class Window {
    private Long window;
    private Thread t1, t2;
    
    public Window() {
         t1 = new Thread(new HandleInput(this), "GLFW_THREAD");
         t2 = new Thread(new Render(this), "OPENGL_THREAD");
         t1.start();
         t2.start();
    }
    
    synchronized public void pollEvents() {
    
        System.out.println("input was handled... ");
        notify();   
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }

    }
    
    synchronized public void renderer() {
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        
        System.out.print("rendering... ");
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("successfully rendered. ");
        notify();
        }
    }

Render.java

public class Render implements Runnable{

Window window;
    public Render(Window window) {
        this.window = window;
    }
    
    @Override
    public void run() {
        window.renderer();
    }

}

HandleInput.java

public class HandleInput implements Runnable{
    private Window window;
    public HandleInput(Window window) {
        this.window = window;
    }
    @Override
    public void run() {
        window.pollEvents();
    }
}

But when I added a new variable named isRenderNeeded and put wait() inside the while statement it by surprise worked. The following code works:

Window.java

public class Window {
    private Thread t1, t2;
    volatile private boolean isRenderNeeded=false;
    
    public Window() {
         t1 = new Thread(new HandleInput(this), "GLFW_THREAD");
         t2 = new Thread(new Render(this), "OPENGL_THREAD");
         t1.start();
         t2.start();
    }
    
    synchronized public void pollEvents() {
    while(true) {
        while(isRenderNeeded)
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        System.out.println("input was handled... ");
        isRenderNeeded = true;
        notify();   
    }

    }
    
    synchronized public void renderer() {
        while(true) {
        while(!isRenderNeeded)
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        
        System.out.print("rendering... ");
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("successfully rendered. ");
        isRenderNeeded = false;
        notify();
        }
    }
    }

Prove me wrong, but I think that it means I'm absolutely nullifing the wait() method and simply do polling there. :/

CodePudding user response：

TL;DR Check out Object#wait Javadocs and J2SE Specs - Threads and Locks chapter to gain a thorough understanding of the multi-threading guarantees that JVM offers, its limitations and how to guard against them.

---

When I read the first version of your code, the are a couple of points that immediately stand out:

1. It assumes an order of execution between t1 and t2, ie it expects that t2:window#renderer calls Window#wait before t1:window#pollEvents calls Window#notify.

In practice, there's a good chance that t1 calls Window#notify before t2 gets a chance to call Window#wait. This can in the worst case cause a deadlock and in the best a missed notify.

2. It assumes that once Window#wait is called, the calling thread is not going to wake up until the other thread calls Window#notify.

"Spurious wake ups" are a common phenomenon in multi-threaded code using native threads (like JVM). It means that, for example, there's a chance that before t1 can call Window#notify, t2 is scheduled, calls Window#wait and then is spuriously woken up and continues execution.

---

The second version of the code pretty much follows "the recommended approach" by JDK developers on how to use Object#wait, ie using an invariant to test if, upon being scheduled, a thread should execute the guarded section.

I don't think you actually need while (true) {...}, but you most certainly do while (isRenderNeeded) {...} and while (!isRenderNeeded) {...}.

CodePudding user response：

Your first chunk of code calls notify, but there is nothing to notify for. No shared state has changed.

Your first chunk of code calls wait, but there is nothign to wait for. There is no shared state that is in the wrong state.

The entire point of wait is to provide an atomic "unlock and wait" operation to avoid the situation where this happens:

1. You acquire a lock to check on shared state.
2. You check whether it is time for you to do something.
3. It is not time yet, so you need to wait.
4. You release the lock so that another thread can change the shared state to indicate that it is time for you to do something.
5. You wait for the other thread.

Now, what happens if the other thread changes the state between steps 4 and 5? You will be waiting for something that already happened.

So wait implements an atomic "unlock and wait", doing steps 4 and 5 with no opportunity for a thread to interfere with the shared state in-between those steps. But that means the lock that gets released during the wait has to protect some shared state that you checked and decided to wait for.

Your first chunk of code doesn't have that. So it has no use for wait. There is no lock protecting shared state that needs to be unlocked at all, and there is no change in shared state to wait for.

Your last chunk of code needs an atomic "unlock and wait" operation. It has shared state (isRenderNeeded) that is protected by a lock (the object is locked because the functions are synchronized), so you need a way to release the lock (so the other thread can access and modify isRenderNeeded) and wait for a signal without the other thread signaling after you unlock but before you wait.

So your first chunk of code makes no sense, waiting for nothing and notifying of nothing. Your second chunk of code uses wait properly to wait for a change of shared state and notify of a change of shared state.