69

According to:

http://www.ibm.com/developerworks/library/j-jtp03304/

Under the new memory model, when thread A writes to a volatile variable V, and thread B reads from V, any variable values that were visible to A at the time that V was written are guaranteed now to be visible to B

And many places on the internet state that the following code should never print "error":

public class Test {
    volatile static private int a;
    static private int b;

    public static void main(String [] args) throws Exception {
        for (int i = 0; i < 100; i++) {
            new Thread() {

                @Override
                public void run() {
                    int tt = b; // makes the jvm cache the value of b

                    while (a==0) {

                    }

                    if (b == 0) {
                        System.out.println("error");
                    }
                }

            }.start();
        }

        b = 1;
        a = 1;
    }
}

b should be 1 for all the threads when a is 1.

However I sometimes get "error" printed. How is this possible?

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
Oleg
  • 6,124
  • 2
  • 23
  • 40
  • 1
    @OliCharlesworth I think he's asking why the various cached values of `b` aren't synchronized to `b=1` after the volatile write/read to `a`. – yshavit May 16 '12 at 14:35
  • 1
    Have you actually run that code and seen "error" printed, with Java 1.5+? – assylias May 16 '12 at 14:37
  • @assylias: I just ran the above code on Java 6, and it did print errors. – JB Nizet May 16 '12 at 14:39
  • @JBNizet I just ran it with Java 7 a few times and did not see error. The beauty of concurrency problems... – assylias May 16 '12 at 14:40
  • I've run it a few times without errors, too. Running 1.6.0_26 on a 64bit dual core on Ubuntu 10.04. – yshavit May 16 '12 at 14:44
  • I see error using jvm build 1.6.0_31-b05 and windows 7 – Oleg May 16 '12 at 15:02
  • Ah, I see what you're saying now. You're saying that when you run it, it doesn't behave as it "should". – Oliver Charlesworth May 16 '12 at 15:03
  • 4
    @OfekRon According to the Java memory model it doesn't matter whether `b` is volatile or not because the write to it is followed by a write to a volatile var, and in the other thread the read of it is preceded by the read of the same volatile var. – Marko Topolnik May 16 '12 at 17:35
  • 17
    This thread is now under discussion at the Java concurrency-interest email list: http://cs.oswego.edu/pipermail/concurrency-interest/2012-May/009440.html – yshavit May 16 '12 at 17:51
  • 4
    Just a quick update from the concurrency-interest list, it looks like this is fixed in the latest Java7: http://download.java.net/jdk7u6/changes/jdk7u6-b14.html (Check out the last entry in the hotspot section. The bug ID links to a bug report with your use case. – yshavit Jun 15 '12 at 17:20
  • The title is deceptive. This has nothing to do with synchronizing to main memory. (Which almost no modern platform does.) – David Schwartz Jun 21 '17 at 22:44
  • @DavidSchwartz The question was about specification, not implementation. [jsr-133-faq](https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html) uses the same terminology. How modern platforms handle shared memory? – Oleg Jul 26 '17 at 20:30
  • @Oleg There is a very important difference. If you look closely, jsr-133 talks about having the same effect as synchronizing to main memory in a hypothetical system. It is absolutely vital to understand that it is not talking about actually doing anything to actual main memory. This is correctly explained in the quote in the OP. It's the title that's extremely misleading and deceptive. – David Schwartz Jul 26 '17 at 21:12
  • @DavidSchwartz If you look at the link I posted earlier which is not the jsr itself but an article co authored by Brian Goetz they use "main memory" in exactly the same way as this title. I think most people would understand that the question is about abstract concepts but if you feel mislead and deceived please change it, you have my blessing. Also you didn't answer my question, can you tell me were to look if I wan't to understand what a modern cpu, operating system and jvm actually do with shared memory? – Oleg Jul 26 '17 at 21:57
  • 1
    @Oleg Thanks. I'll do that. As for your latter question, start by reading about [cache coherence](https://en.wikipedia.org/wiki/Cache_coherence) or about [protocols like MESI](https://en.wikipedia.org/wiki/MESI_protocol). – David Schwartz Jul 26 '17 at 22:02

4 Answers4

34

Update:

For anyone interested this bug has been addressed and fixed for Java 7u6 build b14. You can see the bug report/fixes here

Original Answer

When thinking in terms of memory visibility/order you would need to think about its happens-before relationship. The important pre condition for b != 0 is for a == 1. If a != 1 then b can be either 0 or 1.

Once a thread sees a == 1 then that thread is guaranteed to see b == 1.

Post Java 5, in the OP example, once the while(a == 0) breaks out b is guaranteed to be 1

Edit:

I ran the simulation many number of times and didn't see your output.

What OS, Java version & CPU are you testing under?

I am on Windows 7, Java 1.6_24 (trying with _31)

Edit 2:

Kudos to the OP and Walter Laan - For me it only happened when I switched from 64 bit Java to 32 bit Java, on (but may not be excluded to) a 64 bit windows 7.

Edit 3:

The assignment to tt, or rather the staticget of b seems to have a significant impact (to prove this remove the int tt = b; and it should always work.

It appears the load of b into tt will store the field locally which will then be used in the if coniditonal (the reference to that value not tt). So if b == 0 is true it probably means that the local store to tt was 0 (at this point its a race to assign 1 to local tt). This seems only to be true for 32 Bit Java 1.6 & 7 with client set.

I compared the two output assembly and the immediate difference was here. (Keep in mind these are snippets).

This printed "error"

 0x021dd753: test   %eax,0x180100      ;   {poll}
  0x021dd759: cmp    $0x0,%ecx
  0x021dd75c: je     0x021dd748         ;*ifeq
                                        ; - Test$1::run@7 (line 13)
  0x021dd75e: cmp    $0x0,%edx
  0x021dd761: jne    0x021dd788         ;*ifne
                                        ; - Test$1::run@13 (line 17)
  0x021dd767: nop    
  0x021dd768: jmp    0x021dd7b8         ;   {no_reloc}
  0x021dd76d: xchg   %ax,%ax
  0x021dd770: jmp    0x021dd7d2         ; implicit exception: dispatches to 0x021dd7c2
  0x021dd775: nop                       ;*getstatic out
                                        ; - Test$1::run@16 (line 18)
  0x021dd776: cmp    (%ecx),%eax        ; implicit exception: dispatches to 0x021dd7dc
  0x021dd778: mov    $0x39239500,%edx   ;*invokevirtual println

And

This did not print "error"

0x0226d763: test   %eax,0x180100      ;   {poll}
  0x0226d769: cmp    $0x0,%edx
  0x0226d76c: je     0x0226d758         ;*ifeq
                                        ; - Test$1::run@7 (line 13)
  0x0226d76e: mov    $0x341b77f8,%edx   ;   {oop('Test')}
  0x0226d773: mov    0x154(%edx),%edx   ;*getstatic b
                                        ; - Test::access$0@0 (line 3)
                                        ; - Test$1::run@10 (line 17)
  0x0226d779: cmp    $0x0,%edx
  0x0226d77c: jne    0x0226d7a8         ;*ifne
                                        ; - Test$1::run@13 (line 17)
  0x0226d782: nopw   0x0(%eax,%eax,1)
  0x0226d788: jmp    0x0226d7ed         ;   {no_reloc}
  0x0226d78d: xchg   %ax,%ax
  0x0226d790: jmp    0x0226d807         ; implicit exception: dispatches to 0x0226d7f7
  0x0226d795: nop                       ;*getstatic out
                                        ; - Test$1::run@16 (line 18)
  0x0226d796: cmp    (%ecx),%eax        ; implicit exception: dispatches to 0x0226d811
  0x0226d798: mov    $0x39239500,%edx   ;*invokevirtual println

In this example the first entry is from a run that printed "error" while the second was from one which didnt.

It seems that the working run loaded and assigned b correctly before testing it equal to 0.

  0x0226d76e: mov    $0x341b77f8,%edx   ;   {oop('Test')}
  0x0226d773: mov    0x154(%edx),%edx   ;*getstatic b
                                        ; - Test::access$0@0 (line 3)
                                        ; - Test$1::run@10 (line 17)
  0x0226d779: cmp    $0x0,%edx
  0x0226d77c: jne    0x0226d7a8         ;*ifne
                                        ; - Test$1::run@13 (line 17)

While the run that printed "error" loaded the cached version of %edx

  0x021dd75e: cmp    $0x0,%edx
  0x021dd761: jne    0x021dd788         ;*ifne
                                        ; - Test$1::run@13 (line 17)

For those who have more experience with assembler please weigh in :)

Edit 4

Should be my last edit, as the concurrency dev's get a hand on it, I did test with and without the int tt = b; assignment some more. I found that when I increase the max from 100 to 1000 there seems to be a 100% error rate when int tt = b is included and a 0% chance when it is excluded.

John Vint
  • 39,695
  • 7
  • 78
  • 108
  • But the OP is saying that that's not the behaviour he observes. – Oliver Charlesworth May 16 '12 at 15:08
  • @OliCharlesworth Then he must be on a Java runtime that doesnt conform to the Java 5 memory model, or doing something wrong. I'll test this myself and see if observe the same interaction, I have a feeling I won't – John Vint May 16 '12 at 15:10
  • @JohnVint JBNizet says in the comments he got the unexpected behavior too with Java 6 – assylias May 16 '12 at 15:11
  • That would be the expected behavior yes. Quite surprising if it really does not. Serious bug then – Voo May 16 '12 at 15:13
  • @Voo I ran the same simulation and didn't see it once. If it is happening (which I still have a hard time believing) it is a bug in either his JRE implementation, underlying OS or processor. – John Vint May 16 '12 at 15:14
  • I see the error printed with both Oracle 6u30 an 7u3 on Windows 7 (Intel i7-2600). Funny thing is that if I change the print to ("error" + b) I do see "error1" printed. If I disable the JIT (-Djava.compiler=NONE) I don't see it. – Walter Laan May 16 '12 at 15:16
  • 1
    Is it removing the b == 0 as a JIT optimization? – John Vint May 16 '12 at 15:20
  • Doesn't look like http://pastebin.com/pEcEZ1Sx (although I'm no assembler expert at all :)). Probably just runs slower so it catches up correctly – Walter Laan May 16 '12 at 15:34
  • @WalterLaan thanks for that. I wish I was able to re create this. You have any other VM arg's running? – John Vint May 16 '12 at 15:37
  • 2
    I used -XX:+UnlockDiagnosticVMOptions -XX:+PrintCompilation -XX:+PrintAssembly, running from Eclipse debug (but also get with normal run) with JDK6u30 32bit (on 64bit machine). – Walter Laan May 16 '12 at 15:46
  • @WalterLaan It was the Java 6 + 32 bit version. Got it. This has to be a bug – John Vint May 16 '12 at 15:58
  • @Walter Laan Can you confirm that assembly you posted did in fact print "error" I know for me it took a few times – John Vint May 16 '12 at 19:27
  • I was so free and included the assembly of the whole run method [here](http://pastebin.com/4CNsK2s4). Error is printed for me too, win7 sp1 x64 with JDK7_03 32bit version. Only happens with 32bit. Quite serious bug this, interesting. – Voo May 16 '12 at 19:47
  • @Voo Was there no more assembly that was generated? Your output seems consistent with it not displaying "error", that is unless my conclusion is incorrect. What I have seen is two sets of code compiled. One with the correct output like you have and one with the seemingly incorrect that I have also included. – John Vint May 16 '12 at 20:46
  • @John You're right, it generates more than one version (I think OSR - Or does PrintAssembly actually output the interpreter version as well? Don't think so). The second one is the bugged one. – Voo May 16 '12 at 21:28
  • 2
    I'd bet it's an OSR error, it's not the first OSR error to happen (fav. incl. TieredCompilation w/ c1->c2 and a JVM crash). – bestsss May 16 '12 at 22:32
  • @bestsss Sounds likely. Building the SSA (which I assume is where the trigger for the later wrong CSE lies) correctly on only a partial function does sound complicated to begin with. The thing I find most interesting here is: If even such a (relatively) simple to find bug has existed for *years* by now, why would we assume that the much more complicated semantics hold? – Voo May 16 '12 at 23:11
  • @Voo, this is a compiler mistake, if it involves OSR it's difficult to test (I, myself, never leave a benchmark to run w/ OSR). On x86 with or with volatile it should work properly, if the values are always read. The only way to get it wrong is not using a load but the cached register. I am not sure even if C1 itself is the culprit but C1 got involved so much lately w/ tiered compilation. I suppose CAS on x86 can mask quite a few omissions/bugs. – bestsss May 17 '12 at 11:02
  • @JohnVint: `load of b into tt will store the field locally which will then be used in the if coniditonal` - It does not "store the static one into a local one" they are ints, the local will just have the same value. Secondly, `...which will then be used in the if coniditonal (the reference to that value not tt)...` - Again, they are primitive ints and java is call by value. Thirdly, since they are primitive ints, how will `int tt = b;` // makes the jvm cache the value of b? Can you please clarify? – Farhan stands with Palestine Nov 23 '18 at 22:56
12

Based on the extract from JCiP below, I would have thought that your example should never print "error":

The visibility effects of volatile variables extend beyond the value of the volatile variable itself. When a thread A writes to a volatile variable and subsequently thread B reads that same variable, the values of all variables that were visible to A prior to writing to the volatile variable become visible to B after reading the volatile variable.

assylias
  • 321,522
  • 82
  • 660
  • 783
  • I would +1 this, because it's why I'd think we could never see "error"... except that some people are reporting they do see "error", so somehow this must not apply! – yshavit May 16 '12 at 15:00
  • For the people that never see this error...just to narrow stuff down...what JVM and CPU are you testing on? – cHao May 16 '12 at 15:40
  • @cHao Intel i7-2600 (4 cores) / Windows 7x64 / JDK 1.7.0_03 - Can't replicate the issue. – assylias May 16 '12 at 15:41
  • On OS X, HotSpot(TM) 64-Bit Server VM (build 20.6-b01-415, mixed mode), cannot reproduce. – Marko Topolnik May 16 '12 at 17:55
  • @MarkoTopolnik Try 1.6_32 32 bit – John Vint May 16 '12 at 18:48
  • 2
    @JohnVint Hell yea. With `-d32` it **reliably** reproduces the problem. – Marko Topolnik May 16 '12 at 18:55
  • 1
    @MarkoTopolnik Looking into the assembly it appears (though I can be wrong) that when it fails it is referencing the local store of `tt`. In which 'b'==0 is true. – John Vint May 16 '12 at 18:57
  • 1
    @JohnVint I'm peaking into that thread in [concurrency-interest], it's only 32-bit -client that's causing trouble. I have verified on my machine, with -server I don't see the effect. – Marko Topolnik May 16 '12 at 20:14
  • @MarkoTopolnik Good call. Looks like they are looking into it, I am sure its just a matter of time until they iron it out. – John Vint May 16 '12 at 20:34
  • @JohnVint Got that right, it's a matter of **minutes** -- they already have a workaround cmdline-switch: `-XX:-UseGlobalValueNumbering`. But for them that's just a good diagnostic datum. They seem to be steamrollering over this one! – Marko Topolnik May 16 '12 at 21:03
  • @assylias Can you quickly edit your question? I was meaning to upvote but accidentally down voted and didn't realize it. Shouldnt have done it on my phone :) – John Vint May 17 '12 at 16:01
  • No one would write a real program where they expect `b` to have the latest value, regardless what the spec "guarantees". – Monstieur Dec 23 '13 at 15:19
2

You might want to check out a discussion thread on the concurrency interest mailing list on this question: http://cs.oswego.edu/pipermail/concurrency-interest/2012-May/009440.html

It seems like the problem is more easily reproduced with the client JVM (-client).

sjlee
  • 7,726
  • 2
  • 29
  • 37
-3

in My opinion,The Problem acurred due to Lack of Synchronization :

NOTICE : if b=1 heppens before a=1, and a is volatile while b is not, then b=1 actually updates for all threads only after a=1 is finished (according to the quate's logic).

what heppend in your code is that b=1 was first updated for the main process only, then only when the volatile assignment finished, all the threads b's updated. I think that maybe assignments of volatile are not working as atomic operations (needs to point far, and somehow update rest of refernces to act like volatiles) so this would be my guess why one thread read b=0 instead of b=1.

Consider this change to the code, that shows my claim:

public class Test {
    volatile static private int a;
    static private int b;
    private static Object lock = new Object();


    public static void main(String [] args) throws Exception {
        for (int i = 0; i < 100; i++) {
            new Thread() {

                @Override
                public void run() {
                    int tt = b; // makes the jvm cache the value of b

                    while (true) {
                        synchronized (lock ) {
                            if (a!=0) break;
                         }
                    }

                    if (b == 0) {
                        System.out.println("error");
                    }
                }

            }.start();
        }
        b = 1;
        synchronized (lock ) {
        a = 1;
        }  
    }
}
Ofek Ron
  • 8,354
  • 13
  • 55
  • 103
  • 4
    Unfortunately you are incorrect. There are specific requirements. Take a look at http://g.oswego.edu/dl/jmm/cookbook.html. You will notice in that `Can Reorder` grid the synchronization promises the JMM holds. The important parts are 1. Normal Store cannot be reordered with a subsequent Volatile Store, and 2. Volatile Load cannot be reordered with a subsequent Normal Load. Both this example accounts for – John Vint May 16 '12 at 18:39