11

Please look at this code(taken from Effective Java book)

import java.util.concurrent.TimeUnit;


public class Main {
private static boolean stopReq;
public static void main(String[] args) throws InterruptedException {


    Thread bgw = new Thread(new Runnable()
    {
        public void run(){

        int i = 0;
        while(!stopReq){ i++;}
        }
        });
    bgw.start();
    TimeUnit.SECONDS.sleep(1);
    stopReq = true;

}

}

Why does the bgw thread get stuck in an infinite loop? Is it caching it's own copy of stopReq when it reached the loop? So it never sees the updated value from the other thread?

I understand the solution to this problem would be synchronizing or a volatile variable, but I am curious to why this current implementation doesn't work.

thanks

William
  • 1,837
  • 2
  • 22
  • 36
  • Since there's a race condition I think it behaves as expected, namely "unexpectedly" :-) – Edwin Dalorzo Jan 15 '14 at 11:15
  • 1
    It is not in infinite loop. – Rahul Jan 15 '14 at 11:28
  • 1
    The main lesson for OP: you have changed Effective Java's code in a way which you thought didn't matter, yet it turned out to matter a lot. That's how it is when messing with the borderline cases of the Java Memory Model. – Marko Topolnik Jan 15 '14 at 11:46
  • The Java Language Specification addresses this: https://docs.oracle.com/javase/specs/jls/se11/html/jls-17.html#jls-17.3 – VGR Sep 05 '20 at 12:26

5 Answers5

16

Your explanation is right.

The compiler detects than stopReq is never modified in the loop and since it is not volatile, optimizes the while(!stopReq) instruction to while(true).

Even though the value changes later, the thread does not even read it any more.

Jean Logeart
  • 52,687
  • 11
  • 83
  • 118
  • +1 Here is an article which demonstrates this. http://vanillajava.blogspot.co.uk/2012/01/demonstrating-when-volatile-is-required.html In particular you can see the thread will stop if you change the boolean before the method is JITed or you add a synchronzied block. – Peter Lawrey Jan 15 '14 at 11:27
  • @PeterLawrey Adding a `System.out.println` in the main thread should also make the thread end, shouldn't it? By specification, this will cause a *happens-before* relation between the println in the main thread and the println in the child thread. – Marko Topolnik Jan 15 '14 at 11:30
  • I have tested it on my side and it stops after a second, even without the println in the main thread. That's what I expected, to be honest, because I can't imagine HotSpot proving that i won't acquire `out`'s lock anywhere else. – Marko Topolnik Jan 15 '14 at 11:33
  • Whatever you have given explanation, according to that it should be infinite loop. But it is not so. Earlier in main stopReq = false; was there that's why it was in infinite loop. – Rahul Jan 15 '14 at 11:34
  • 1
    I tested it without the System.out.println and it gets stuck in the loop forever. With the println it gets out after 1 second. – William Jan 15 '14 at 11:35
  • @Rahul Remove the println and you'll see it goes into infinite loop, even if you set the boolean to true later. – Marko Topolnik Jan 15 '14 at 11:36
  • @MarkoTopolnik yes it is in infinite loop when not using println. Why it is so? – Rahul Jan 15 '14 at 11:40
  • 1
    @Rahul Because `println` is a `synchronized` method. – Marko Topolnik Jan 15 '14 at 11:40
  • I don't think that JIT will replace `while(!stopReq)` with `while(true)`. The _stopReq_ variable is a shared one and the escape analysis would detect this. – JB- Jan 15 '14 at 11:46
  • @J.B You are wrong. The Java Memory Model doesn't care about shared variables, it cares about *happens-before* relationships, of which there are none without a `volatile` or a `synchronized` somewhere on the code path. Anyway, you have your Java runtime, run it and see for yourself. – Marko Topolnik Jan 15 '14 at 11:47
  • @J.B BTW escape analysis has nothing to do with this, anyway. It is concerned with objects created within a method and proving that they cannot escape its *dynamic scope*. – Marko Topolnik Jan 15 '14 at 11:53
  • @MarkoTopolnik Now you are mixing the java memory model and JIT optimizations. While the result is the same (infinite loop) the ways you can get there are different. – JB- Jan 15 '14 at 12:07
  • Yup, the only thing the JIT has to detect here to be allowed to hoist the load of `stopReq` out of the loop is that no code inside the loop can changes its value to true (this has indeed nothing to do with the JMM as long as there are no synchronization primitives such as volatile, synchronized, etc. involved). In such a trivial case the JIT will *definitely* hoist the load out - heck we only got this post because it does exactly that ;) – Voo Jan 15 '14 at 12:08
  • @J.B I am not *mixing* them, that's hilarious :) What do you think the JIT optimizations must abide by? a) Holy Bible b) Java Memory Model? – Marko Topolnik Jan 15 '14 at 12:09
  • @Voo Indeed, the load is hoisted - you can check the generated assembly code using `-XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly` flags. The field is tested only once, immediately after load. This means that from time to time the test method might finish correctly. – JB- Jan 15 '14 at 12:30
  • FWIW, `PrintAssembly` is a rather blunt tool... it will inundate the output with irrelevant data. Better use `CompileCommand=print,*MyClass.myMethod` – Marko Topolnik Jan 15 '14 at 12:32
  • @Voo This sentence of yours is self-contradictory: `this has indeed nothing to do with the JMM as long as there are no synchronization primitives such as volatile, synchronized, etc. involved` It is the JMM which stipulates the "as long as" part, so it has *everything* to do with it. – Marko Topolnik Jan 15 '14 at 12:35
  • @Marko Fair point. What I meant to say is that the JMM specifies the situations in which its rules apply and this scenario doesn't fall under them (so the rule that specifies when it applies still applies, ah you get the idea). I know about `PrintAssembly`, although I'd just look at the nodes generated by the JIT during different phases - not only easier to read, but also gives me the reasoning of the JIT. I think only works with debug versions of HotSpot though, not sure. – Voo Jan 15 '14 at 12:42
  • 2
    @Voo But my point is, JMM's rules apply *everywhere* and without those precise rules, the JIT compiler would never have the entitlement to hoist a static variable load out of the loop. So it's actually the reverse: precisely *due* to JMM's rules is this optimization granted. I know you and I understand that; I am writing this for the greater public reading the discussion. – Marko Topolnik Jan 15 '14 at 12:44
  • @MarkoTopolnik Yes, the optimization is *allowed* but not enforced. Try running the example with `-Xcomp` - the different level of optimization changes the program behavior. But the behavior still obeys JMM. – JB- Jan 15 '14 at 12:48
  • @J.B Absolutely true. But without the JMM there to specify that it is *allowed*, it wouldn't be allowed because, as you have already noted, that boolean is a shared variable. – Marko Topolnik Jan 15 '14 at 12:49
  • @Marko We are clearly on the same page here. But you are right - due to work when I think about "JMM rules" I'm thinking memory orderings and all the other details, but clearly the most important rule is the one that specifies when these apply. Raymond Chen would put this in a variant of his "[Looking through kernel-colored glasses](http://blogs.msdn.com/b/oldnewthing/archive/2011/05/12/10163578.aspx)" category I guess. – Voo Jan 15 '14 at 12:54
  • 1
    @Voo I know exactly what you mean---I do it too, often. I replace the official meanings with those that make sense for me day-to-day. In this particular case, you have substituted "JMM" for "happens-before" because happens-before is really the essence of the JMM. – Marko Topolnik Jan 15 '14 at 12:56
1

You should read more about Java Memory Model to better understand all the implications.

Shortly, the stopReq variable not being volatile or included in a synchronized block gives the VM freedom to use an optimized local storage (eg. registers etc) which is not guaranteed to propagate changes immediately across the threads.

When you declare the variable as volatile the VM will make sure that after each variable write a "memory write barrier" is inserted which will force all the local changes to be spilled to the real memory location thus making it visible to all the other threads (the same barrier is placed at the end of a synchronized block eg.)

JB-
  • 2,615
  • 18
  • 17
0

I tested this out, and no, the variables are the same. The example also compiles for me.

The error is here:

Your while loop goes on, as long as !stopReq is true, that means stopReq is false. And after 1 sec you set stopReq to false - this changes nothing. If you set it to true, !stopReq will become false and your loop will end.

Anedar
  • 4,235
  • 1
  • 23
  • 41
0

Make stopReq to true, then it will be stopped. You are again setting the stopReq to false, due to that while loop condition is true always and it is in infinite loop.

Rahul
  • 3,479
  • 3
  • 16
  • 28
0

To be very specific about your query, to take full advantage of the performance of modern multiprocessor hardware, in absence of synchronization, JVMs allowed to permit compiler to re-order operations and cache values in registers and in processor specific caches. As main thread writes to stopReq without synchronization so because of reordering and caching the BTW thread might never see the written value and loop forever.

When you use synchronization or volatile they guarantee VISIBILITY and force compiler not to cache and flush changes to main memory.

Mak
  • 596
  • 5
  • 10