3

The answer for this program will have to be "Changing done" after 5 secs, but I get "Changing done" and "DONE". I do not have the getDone method as synchronized. Any ideas what am I doing for the thread to finish processing. 

public class Main {
    private static boolean done = false;
    private static int count;

    public static void main(String[] args)throws InterruptedException {
        new Thread(() -> {
            while (!getDone()) {
                count = count + 1;
            }
            System.out.println("DONE!!!!");
        }).start();
            Thread.sleep(5000);
        System.out.println("Changing done");
        synchronized (Main.class) {
            done = true;
        }
    }

    public static boolean getDone() {
        return done;
    }
}
sathish_at_madison
  • 823
  • 11
  • 34
  • 2
    Your program is doing exactly what I would expect it to. Were you expecting your anonymous `Runnable` to not finish? – CodeBlind Oct 17 '16 at 19:20
  • @CodeBlind: access to `done` is not properly synchronized, the call to `getDone()` may not see the current value. – user140547 Oct 17 '16 at 19:21
  • 1
    The worst that could happen here is the worker thread may not see the change in `done` immediately, but it certainly will eventually. Since only one thread ever changes `done`, you could get away with using the `volatile` keyword in its declaration and do away with the `synchronized` block entirely. But that's all water under the bridge - I still don't understand what the OP is getting at. He seems surprised that the worker thread is printing `DONE!!!!`. – CodeBlind Oct 17 '16 at 21:49
  • 1
    @CodeBlind: If you don't use volatile or synchronize or some other synchronizing mechanism the thread may never see the updated value. See http://stackoverflow.com/questions/106591/do-you-ever-use-the-volatile-keyword-in-java – user140547 Oct 18 '16 at 05:49
  • @CodeBlind: As my answer explains, OP's question (correctly) notes that that code is not properly synchronized, yet it still works. But broken code still works in many cases, it does not mean that the write guaranteed to be invisible. – user140547 Oct 18 '16 at 05:52
  • 1
    Got a perfect “looping forever” behavior with your program right in the first run. It obviously depends on the environment… – Holger Oct 18 '16 at 10:31

2 Answers2

5

If you don't synchronize access to done properly, it means your code may fail, i.e. the thread may not see the updated value.

It does not mean the value is guaranteed not visible unless synchronizing correctly. So, in many cases the write to done is still visible (the fact that broken code still works in many cases makes concurrent programming harder). It is just not guaranteed to work in every case.

user140547
  • 7,750
  • 3
  • 28
  • 80
  • The change to done is performed in a synchronized block. It will be visible. This is stated in the [JLS](https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4) under the memory model. Do you have a specific reference to demonstrate why the thread may not see the updated value? Or maybe it is in the reference I have included, but after a cursory reading, I think the changes performed to done will be visible. – matt Oct 18 '16 at 08:09
  • @matt: not only the write has to be synchronized, but also the read. You don't need the JLS for that. see for example, http://stackoverflow.com/questions/9196723/learning-java-use-of-synchronized-keyword quote from the accepted answer (emphasis mine): when a synchronized method exits, it automatically establishes a happens-before relationship with **any subsequent invocation of a synchronized method for the same object** – user140547 Oct 18 '16 at 08:32
  • I am not saying there wont be a race condition. The synchronized guarantees that the changes will be visible across threads. It is similar to declaring the variable volatile. – matt Oct 18 '16 at 09:12
  • 1
    @matt: no it’s not. When the variable is declared `volatile`, both, writes and reads accessing the same `volatile` variable establish a *happens-before* relationship. Performing a `synchronized` on the write only doesn’t guaranty anything. – Holger Oct 18 '16 at 10:26
  • @Holger I don't think happens-before is the relevant criteria here. 17-3 gives an equivalent example. `The compiler is free to read the field this.done just once, and reuse the cached value in each execution of the loop.` Which would agree with both of you. It has been my understanding that a synchronized statement will cause a synchronization of the memory across threads. Which sounds wrong, unless I find the statement that led me to that conclusion. – matt Oct 18 '16 at 10:47
  • 3
    @matt: it does. But only for established *happens-before* relationships, i.e. the writes made by a thread leaving a `synchronized` block are guaranteed to be read by another thread subsequently entering a `synchronized` block, when synchronizing on the same object. Otherwise, how would the thread having the cached value ever know that another thread executed a `synchronized` block? – Holger Oct 18 '16 at 10:50
1

I do not have the getDone method as synchronized. Any ideas what am I doing for the thread to finish processing.

As you mention, there is no explicit memory synchronization between the done as seen by your spinning thread and the main thread. Although the main thread crosses a write memory barrier when exiting the synchronized block, there is no explicit read memory barrier crossed by the spinning thread.

However, there are many ways for a thread to see updated information. If the operating system swaps the thread out of its running processor, the cached memory may be lost so when the thread gets swapped back it, it will request done from central memory and will see the update.

Also, although your example code doesn't show it, if you make calls to other synchronized methods (such as System.out.println()) or cross other memory barriers (accessing another volatile field), then this will also cause done to be updated.

Gray
  • 115,027
  • 24
  • 293
  • 354