1

Why this code successfully finishes when method get() is marked as synchronized despite the fact that field value is not volatile? Without synchronized it runs indefinitely on my machine (as expected).

public class MtApp {

    private int value;

    /*synchronized*/ int get() {
        return value;
    }

    void set(int value) {
        this.value = value;
    }

    public static void main(String[] args) throws Exception {
        new MtApp().run();
    }

    private void run() throws Exception {
        Runnable r = () -> {
            while (get() == 0) ;
        };
        Thread thread = new Thread(r);
        thread.start();
        Thread.sleep(10);
        set(5);
        thread.join();
    }
}
Impurity
  • 1,037
  • 2
  • 16
  • 31
Yuriy Kiselev
  • 248
  • 3
  • 8
  • Without correct synchronization, program results are indeterminate. The program could run and finish correctly, or it might not, it's all up to the whims of the JVM implementation (and that implementation could change at any time). – markspace Sep 08 '18 at 19:14

3 Answers3

2

Synchronization forces this.value = value to happen before get().

It ensures the visibility of the updated value.

Without synchronization, there is no such guarantee. It might work, it might not.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
1

@Andy Turner is partly correct.

The addition of the synchronized on the get() method is affecting the memory visibility requirements, and causing the (JIT) compiler to generate different code.

However, strictly speaking there needs to be a happens before relationship connecting the set(...) call and the get() call. That means that the set method should be synchronized as well as get (if you are going to do it this way!).

In short, the version of your code that you observed to be working is NOT guaranteed to work on all platforms, and in all circumstances. In fact, you got lucky!


Reading between the lines, it seems that you are trying to work out how Java's memory model works by experimentation. This is not a good idea. The problem is that you are trying to reverse engineer an immensely complicated black box without enough "input parameters"1 for you to vary to cover all potential aspects of the black boxes behavior.

As a result the "learning by experiment" approach is liable to leave you with an incomplete or mistaken understanding.

If you want a complete and accurate understanding, you should start by reading about the Java Memory Model in a good text book ... or the JLS itself. By all means, use experimentation to try to confirm your understanding, but you do need to be aware that the JMM specifies (guarantees) only what happens if you do the right thing. If you do the wrong thing, your code may work anyway ... depending on all sorts of factors. Hence, it is often difficult to get experimental confirmation that a particular way of doing things is either correct or incorrect2.

1 - Some the parameters you would need don't actually exist. For example, the one that allows you to run Java N for N > 12, or the one that allows you to run on hardware that you don't have access ... or that doesn't yet exist.

2 - As is illustrated by your example. You are getting the "right" answer, even though the code is wrong.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • This is a very detailed and correct explanation and should be marked as the correct answer. – khachik Sep 09 '18 at 01:23
  • Thank you for thorough answer @Stephen! I do know this code is broken (with or without synchronized on getter) and I definitely wouldn't use something like that in production. – Yuriy Kiselev Sep 09 '18 at 18:36
0

For starters, either value needs to be volatile or both get and set need to be synchronized for this to be correct.

JLS 17.4.5:

An unlock on a monitor happens-before every subsequent lock on that monitor.

It is possible for value to be set to 5 prior to the lock being released, which puts it before the happens-before edge and makes it available the next time the lock is acquired.

It should be noted that such guarantees are fragile and depending on the thread scheduler, may not exist at all. On platforms with weaker synchronization models, you may not see the same effects you see here.


See also: Loop doesn't see changed value without a print statement

Strange behavior of a Java thread associated with System.out