2

Please see the code below：

private  static boolean flag=true; // main thread will call flag=false

private final static Object lock=new Object(); // lock condition

public static void thread1(){

    while (flag){

        synchronized (lock){
            // some work
        }

    }

}


public static void main(String[] args) throws Exception {

    Thread t1=new Thread(()->{
        thread1();
    });
    t1.start();
    Thread.sleep(1000);
    flag=false;

    // The program can stop normally

}

No matter at any time , When one thread entering the synchronized block, will the value of the variable flag be loaded from main memory?

Thank you for your detailed explanation, because I am not sure if the flag has a happend-befores relationship. Literally, the flag is not in the synchronized block.

Update1：

I know that using volatile can and I also know how to write the right code,, but I want to know now if there is no volatile keyword. Whether synchronized can guarantee visibility. Note: the flag variable is not in synchronized block.

Update2：

I updated the code again, the code on my win10+JDK8 system can stop normally, do you think it is correct or accidental, because it is not tested on all hardware systems, so I need theory to guide。

Focus on the question:

whether the loop condition (flag variable) has a happen-before relationship with the synchroized block inside the loop，If it has a happen-before relationship, jvm whether ensure that the flag variable is loaded from main memory even if the flag variable is not in the synchronized block.

If every one think there is no happen-before relationship, then how do you explain that when I remove the synchronized block, the code will loop indefinitely. When I add it, it will stop normally. Is this just an accident?

Qin Dong Liang
  • 415
  • 3
  • 12
  • Could just make the flag `volatile` :} It's a somewhat pedantically interesting question, though. – user2864740 Jul 09 '18 at 03:10
  • I don't think so. `synchronized` only locks an object to prevent another thread from entering (which would block those threads). What you need is to make the `flag` `volatile`. – Jai Jul 09 '18 at 03:15
  • 1
    Basically yes (I'm contradicting Jai here). In Java, the ideas of mutex and memory visibility are packed into the same operations. So `volatile` works here, but so does `synchronized` and they basically do the same thing. Note that your example is bad because `flag` is public. Normally you make the field `private` so that calling code must go through the synchronized block for any access. Then the field is considered thread safe. – markspace Jul 09 '18 at 03:20
  • Also a "detailed explanation" is far too broad to cover here. Get the book [*Java Concurrency in Practice* by Brian Goetz](http://jcip.net/) (http://jcip.net/) for the full story. It's the best book on Java threading and concurrency. – markspace Jul 09 '18 at 03:22
  • Possible duplicate of [Difference between volatile and synchronized in Java](https://stackoverflow.com/questions/3519664/difference-between-volatile-and-synchronized-in-java) – Jai Jul 09 '18 at 03:30
  • I have updated the question, please focus on the updated content. – Qin Dong Liang Jul 09 '18 at 03:31
  • Read the link, I think it addresses your problem. @markspace I found an answer that describes more in detail what you said. – Jai Jul 09 '18 at 03:31
  • @Jai Please see the updated content, I know the theory, but the theory of this situation is not described, so this question is not a duplicate question, thank you – Qin Dong Liang Jul 09 '18 at 03:38
  • The String literal `lock` and `flag` are two different references, why would you think the `synchronized` block would make `flag` implicitly volatile when it's not even inside the block? – Jai Jul 09 '18 at 03:51
  • Updated my answer below, the code you added definitely does not work. – markspace Jul 09 '18 at 03:52
  • Thanks to every friend for the discussion, please see the update again. – Qin Dong Liang Jul 09 '18 at 04:03
  • [IMPORTANT : Please do not use String as lock](https://stackoverflow.com/a/463437/5217712). – Enzokie Jul 09 '18 at 04:26

2 Answers2

3

OK looking a little more closely at your code, what you have is not enough. The access to a shared field is outside of your synchronized block, so no it does not work.

In addition, Java requires that both the read and the write of shared memory be "synchronized" somehow. Using the synchronized keyworld, that usually means you need to use it on both the read and the write, and you did not show the write.

And in addition to that, the "lock" that you use for a given set of fields or shared memory must be the same lock for both the read and the write. Seriously, volatile is a lot easier here, and the API in java.util.concurrent is even easier and recommended. Don't try reinventing the wheel.

private static boolean flag = true; // must use 'resetFlag'

public void resetFlag() { synchronized( "lock" ) {flag = false;} }

public boolean getFlag() { synchronized( "lock" ) {return flag;} }

public void thread1() {
    while ( getFlag() ){
        synchronized ("lock"){
            // other work
        }
    }
}

public static void main(String[] args) throws Exception {

    Thread t1=new Thread(()->{
        thread1();
    });
    t1.start();
    Thread.sleep(1000);
    resetFlag();

    // The program can stop normally

}

I think the above has the required changes.

Regarding your second update: the code on my win10+JDK8 system can stop normally Yes it can. Memory visibility is not guaranteed, but it is not prohibited. Memory can be made visible for any reason, even just "accidentally." On Intel platforms, Intel has a QPI bus which exchanges memory update information at high speed, bypassing the memory bus. However even that can be got around by software, so it's best to just put the synchronization where needed (Hint: look at AtomicBoolean.)

markspace
  • 10,621
  • 3
  • 25
  • 39
  • And I've updated for your second update. @QinDongLiang – markspace Jul 09 '18 at 04:05
  • I know how to write the right code, and now our goal is to discuss whether my code above is correct or accidental. You can test my code above and then how should you explain it. – Qin Dong Liang Jul 09 '18 at 04:08
  • I said in my reply that it is "accidental." That's the only reason it works. @QinDongLiang – markspace Jul 09 '18 at 04:09
  • If I remove the sync block, my code will loop indefinitely, I think they should be related, can you explain why this is? @markspace – Qin Dong Liang Jul 09 '18 at 04:15
  • No. What you are doing is out side of the scope of the language specification (literally, read the spec.) It only works by accident, changes to the JVM or `javac` could change the behavior of your program. – markspace Jul 09 '18 at 04:30
  • I'm actually not sold that this wouldn't work per spec if the `flag=false` were also synchronized on `"lock"`; i.e., I think the block inside the loop creates a *happens-before* w.r.t. examining the loop conditional. – chrylis -cautiouslyoptimistic- Jul 09 '18 at 04:47
  • if flag=false were synchronized then of course it would work; if it wasn't, then no there is no happens-before –  Jul 09 '18 at 05:40
  • The question is whether this flag variable has a happen-before relationship with the synchronized block. This is a confusing problem. If it has a happen-before relationship, jvm will ensure that the flag variable is loaded from main memory even if the flag variable is not in the synchronized block. – Qin Dong Liang Jul 09 '18 at 05:47
  • 1
    1. happens before only applies to actions 2. as the answerer posted the previous 3 times, there are no happens before relationships whatsoever in your code 3. synchronizing on a string is almost totally useless and will probably be optimized away anyway so the question is moot in the first place –  Jul 09 '18 at 05:58
  • @xTrollxDudex ok，If you think there is no happen-before relationship, then how do you explain that when I remove the synchronized block, the code will loop indefinitely. When I add it, it will stop normally. Is this accidental? – Qin Dong Liang Jul 09 '18 at 06:18
  • The absence of a happens-before relationship does not necessarily mean that your code will not work. By having insufficient synchronization, you are opening yourself up to portability bugs and relying on the hardware to keep you safe (it won't). It is fallacious to believe that the results you are seeing are sound; you are scratching your head now wondering why your code works for no reason, but I can guarantee you that you will also be scratching your head in the future wondering why the same code doesn't work in a different circumstance –  Jul 09 '18 at 06:26
  • I will also add that relying on mere analogies such as fetch from main memory to explain your results are equally fallacious and you should be careful when relating the guarantees of the JMM (or lack thereof in this particular instance) back to the hardware –  Jul 09 '18 at 06:34
  • And if I have failed to make it sufficiently clear enough in the past 3 comments, refer to this in-depth answer to the same "serendipitous" synchronization giving the correct answer here: https://stackoverflow.com/a/25425131/3308999 –  Jul 09 '18 at 06:44
  • @xTrollxDudex What you mean, the above execution result is accidental. In other words, the execution result is unpredictable. Is this true? – Qin Dong Liang Jul 09 '18 at 06:51
  • No, it is not accidental, but it is incorrect, the method you are using to achieve the result –  Jul 09 '18 at 06:52
  • @xTrollxDudex Thank you for the information provided， I have know the reason for it, although my code above can stop normally, the result is predictable, but it is not a correct code habit。 – Qin Dong Liang Jul 09 '18 at 07:33
1

Thanks to the information provided by @xTrollxDudex and @markspace ，The code in the loop section is observed from the jvm level, If there is no happens-before relationship and the code may be optimized from :

       while (flag){

        synchronized (lock){
            // some work
        }

    }

to :

      if(flag){

        while (true){

            synchronized (lock){
                //some work
            }

        }

    }

To ensure thread visibility, we need to avoid this optimization, such as through the volatile keyword or other synchronization strategies. The appearance of the sync block in the loop is similar to the function of the enhanced volatile keyword, which guarantees the visibility of the variable in front of it, so when we loop into the sync block for the second time, we can see it latest. The change, which is why the loop can stop normally. It looks fine, but it's not the right synchronization method, so don't do it.

For a detailed explanation, please see a similar question in here

Qin Dong Liang
  • 415
  • 3
  • 12