1

Please take a look at the code below. I know why I don't use the "synchronized" part, the loop will never break, but what I don't know is why I use the "synchronized" part, the loop breaks. It seems don't match the Happens-Before principle.please help me out, thank you very much.

public class Test implements Runnable {

    private String s = "continue";

    @Override
    public void run() {

        while (!"break".equals(this.s)) {
            //synchronized (this){
            //
            //}
        }

        System.out.println("loop has been breaked!");
    }

    public static void main(String[] args) throws InterruptedException {
        Test test = new Test();

        Thread t1 = new Thread(test);
        t1.start();

        TimeUnit.SECONDS.sleep(1);
        test.s = "break";
    }

}
Richard H.
  • 227
  • 1
  • 3
  • 9

1 Answers1

0

This is not a case of happens-before, it's a visibility issue. Making s volatile would also work, making the value change visible to the thread instead of a cached value being used.

From the POV of the Java Memory Model, using synchronized the way you're using isn't correct. You would also need to sync on test when assigning test.s = "break";. However your sync code works because of the underlying x86 architecture, which can affect the visibility even with a NOOP synchronized block.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • Thx, I know add volatile will work, what makes me confuse is will add the syn part makes the s value-change visible. Don't add the syn part the s value-change is invisible. – Richard H. Oct 03 '19 at 06:58
  • @RichardH. it's a side-effect really. On a different architecture it might work differently. – Kayaman Oct 03 '19 at 06:58
  • Could you please tell this more specific, – Richard H. Oct 03 '19 at 07:00
  • If I add the sync part out of the while-loop, it will not break either. – Richard H. Oct 03 '19 at 07:03
  • 1
    @RichardH. that makes sense, since then the thread is constantly inside the sync block. But this is very nitty gritty detail involving bytecode, x86 opcodes and all sorts of things that are too long to explain here. Not to mention that I'm not the best to give the technically correct answer. A simplified answer is "those are side effects, and your program is technically wrong, but happens to work due to the x86 architecture". – Kayaman Oct 03 '19 at 07:09
  • Maybe I don't make myself clear, I know how to make the s visible, what I don't know is why add sync-part the var s is visible, why don't use cache – Richard H. Oct 03 '19 at 07:11
  • @RichardH. I explained it. It's a complicated side-effect. If you're not familiar with the inner workings of the x86 architecture, consider it to be magic. – Kayaman Oct 03 '19 at 07:12
  • Maybe the code doesn't make sense, I just wanna make the code elegant and simple. – Richard H. Oct 03 '19 at 07:15
  • @RichardH. you're confusing me. Do you want to fix your code or understand the inner workings of synchronization? If you want to fix the code, make the variable `volatile`. – Kayaman Oct 03 '19 at 07:15
  • Thank you very much for your help, I will go deeper on this question. – Richard H. Oct 03 '19 at 07:16
  • @RichardH. if you're interested in the memory model, you can start here https://stackoverflow.com/questions/1850270/memory-effects-of-synchronization-in-java – Kayaman Oct 03 '19 at 07:18