2

For the following code snippet:

        Point p = new Point(5, 5);
        Thread t = new Thread(() -> {
            while (p.x == 5) {
                try {
                    Thread.sleep(10);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
            System.out.println("Thread detected that x is changed to " + p.x);
        });
        t.start();
        Thread.sleep(10000);
        p.x = 10;
        System.out.println("Main thread changed x to 10");

I get the output as follows:

Main thread changed x to 10
Thread detected that x is changed to 10

But for the following code:

        Point p = new Point(5, 5);
        Thread t = new Thread(() -> {
            while (p.x == 5) { }
            System.out.println("Thread detected that x is changed to " + p.x);
        });
        t.start();
        Thread.sleep(10000);
        p.x = 10;
        System.out.println("Main thread changed x to 10");

I get only:

Main thread changed x to 10

Why is that happening?

Youssef13
  • 3,836
  • 3
  • 24
  • 41

1 Answers1

1

Probably because of hoisting.

This code

 Thread t = new Thread(() -> {
            while (p.x == 5) { }
            System.out.println("Thread detected that x is changed to " + p.x)

is being changed to this code by the compiler. The compiler has no idea that p.x can change in an empty loop, so it optimizes.

 Thread t = new Thread(() -> {
            if (p.x == 5) {
               while(true) {}
            }
            System.out.println("Thread detected that x is changed to " + p.x)

If Point is declared a volatile type, it should work. This will ensure that the most recent change of p will be seen and the compiler will compile accordingly.

Try this with and without the volatile statement.

import java.awt.Point;

public class Hoisting {

    volatile Point p = new Point(5,5);
    public static void main(String[] args) {
        new Hoisting().start();
    }
    public void start() {
        Thread t = new Thread(() -> {
            while (p.x == 5) { }
            System.out.println("Thread detected that x is changed to " + p.x);
        });
        t.start();
        try{
        Thread.sleep(5000);
        } catch(InterruptedException ie){}
        p.x = 10;
        System.out.println("Main thread changed x to 10");
    }

}
WJS
  • 36,363
  • 4
  • 24
  • 39
  • How can I see what the compiler changes my code to? – Youssef13 Apr 16 '20 at 15:11
  • it doesn't "optimise" it on my machine – Andrew Tobilko Apr 16 '20 at 15:18
  • @WJS Your analysis is ggod, but that'a not even the main problem. The real problem is not what your compiler change code to. The problem is memory barrier. Even if no optimization were done by the compiler, the same problem could occur. When using multiple threads, the memory sharing is not simple. The thread has its own memory stack, when it copies variables it uses. If another thread modify variables, it won't be copied in the first thread memory buffer. Threads *can* share memory. But they do not do so by design. That's why synchronize and volatile keywords exist in Java. – amanin Apr 16 '20 at 15:21
  • @AndrewTobilko well this is what I read about several years ago. So the situation may be different from compiler to compiler. But using volatile to get the most updated value is used is the solution. – WJS Apr 16 '20 at 15:23
  • Yes, `volatile` fixed the problem. But the reasoning of @amanin seems to be more convincing to me. Anyway, thanks everyone :) – Youssef13 Apr 16 '20 at 15:25
  • @Youssef13 It seems more convincing because it is actually the cause of the problem. I don' think @WJS answer is correct here. My only problem is that I cannot find out why this behavior occurs. In theory the first version should not do the print either because `Thread.sleep` is not causing a memory barrier as stated in the [specification](https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.3). Hmm ... – akuzminykh Apr 16 '20 at 15:30
  • 1
    My answer may be correct for instance fields but not local variables. Anyway, this was the explanation of this behavior by Joshua Bloch, in Effective Java Programming. I did not check the byte code so it was an assumption on my part. – WJS Apr 16 '20 at 15:33
  • 1
    @Youssef13 *FYI:* [Here](https://stackoverflow.com/questions/42676751/thread-sleep-makes-compiler-read-value-every-time) is another post that states the exact same problem. – akuzminykh Apr 16 '20 at 15:33
  • @WJS Yeah, no offense, I guess it could be the solution as well, I just don't think the compiler is fault here. I think it's the caching/visibility of the read values. This is also indicated by the fact that `volatile` fixed the problem. – akuzminykh Apr 16 '20 at 15:39
  • @akuzminykh None taken at all. I am not an expert on memory models. – WJS Apr 16 '20 at 15:40
  • @akuzminykh, I just started wondering. If the thread "copies" the variables it uses to its "own" memory stack. Why the sleep or even a print statement make things work correct without the `volatile` keyword? Do you have any explanation for that? – Youssef13 Apr 16 '20 at 18:29
  • @Youssef13 This is exactly what I meant. It seems strange to me that the print goes through, too. I can't tell you what the reason is. I've done some research but couldn't find a reason for that, except that this could depend on the platform, compiler version and other hardware related things. Anyways, I've posted two links that contain potential explanations. – akuzminykh Apr 16 '20 at 19:22