Item 66: Synchronize access to shared mutable data

Question

I am now reading 《effective Java》 and meeting a confusion.

For code 1 (java8) :

public class StopThreadTest {

    private static Boolean stopRequest = false;

    public static void main(String[] args) throws InterruptedException {
        new Thread(()->{
            int i = 0;
            while (!stopRequest) {
                i++;
                //System.out.println("i: " + i);
            }
        }).start();

        TimeUnit.SECONDS.sleep(1);
        stopRequest = true;
    }
}

the program never terminates.

For code 2(java8):

public class StopThreadTest {

    private static Boolean stopRequest = false;

    public static void main(String[] args) throws InterruptedException {
        new Thread(()->{
            int i = 0;
            while (!stopRequest) {
                i++;
                System.out.println("i: " + i);
            }
        }).start();

        TimeUnit.SECONDS.sleep(1);
        stopRequest = true;
    }
}

Just adding System.out.println(), the program run about 1 second.

Can anybody tell me why?

Is this an official example in effective Java? If so: which edition? — Turing85, Sep 06 '17 at 09:55
@Turing85 certainly not in my edition of the book. In fact, the book contradicts this example, saying that both reads and writes of `stopRequest` must be synchronized. Not just some random method whose internal implementation just happens to be synchronized. — Klitos Kyriacou, Sep 06 '17 at 09:59
in second edition, the author use code 1 to address that both reads and writes of stopRequest must be synchronized. I just add add a System.out.println to debug it, just as code 2. — Dustin, Sep 06 '17 at 10:13

Kayaman · Accepted Answer · 2017-09-06T09:52:30.750

4

System.out.println() is synchronized, removing the visibility issues with the original code. Without it, the thread can use its cached value of stopRequest and keep on running, but when println() is involved, caches are flushed and the modified value can be seen.

From PrintStream.println(String x)

synchronized (this) {
    print(x);
    newLine();
}

Note that this is a side-effect only. It explains the difference in behaviour, but it's not something you can rely on for correct functionality of code.

edited Sep 06 '17 at 09:52

answered Sep 06 '17 at 09:19

Kayaman

72,141
5
83
121

2

For the reason explained from Kayaman, if you define stopRequest as volatile, you avoid the cache problem and the behaviour will be the same for both code: after one second the program will stop. – Valerio Emanuele Sep 06 '17 at 09:27
2

I am not quite sure this is correct. While `System.out.println(...)` is `synchronized`, I see no reasoning any reads or writes to `stopRequest` should be `synchronized`. Can you give some JLS refernce to back your statement? – Turing85 Sep 06 '17 at 09:32
1

@KDavid-Valerio This means the code only works because of a side effect of println()`?! I'd be very interested in some links to specs too. – blafasel Sep 06 '17 at 09:35
@blafasel, correct. I think that these question/answers are useful https://stackoverflow.com/questions/20552945/thread-caching-and-java-memory-model – Valerio Emanuele Sep 06 '17 at 09:38
@Turing85 I'm not sure what you mean. It doesn't guarantee that `stopRequest` would be visible, but in this particular scenario, OS and platform, `stopRequest` gets flushed from cache as a side-effect. – Kayaman Sep 06 '17 at 09:56
Thank you a lot, I did not know that System.out.println() is synchronized – Dustin Sep 06 '17 at 09:56
@Dustin this can be problematic when an application isn't working due to concurrency/visibility issues, then you add a `System.out.println` to debug it, and suddenly it works just fine. – Kayaman Sep 06 '17 at 10:01
1

@Kayman I am quite sure your answer lacked the last sentenceatn the point in time I wrote my comment :) all is fine now. – Turing85 Sep 06 '17 at 10:04
@Kayaman Thank you for your advice, I will pay attention to it. – Dustin Sep 06 '17 at 10:07

Item 66: Synchronize access to shared mutable data

1 Answers1