140
votes

I know that compound operations such as i++ are not thread safe as they involve multiple operations.

But is checking the reference with itself a thread safe operation?

a != a //is this thread-safe

I tried to program this and use multiple threads but it didn't fail. I guess I could not simulate race on my machine.

EDIT:

public class TestThreadSafety {
    private Object a = new Object();

    public static void main(String[] args) {

        final TestThreadSafety instance = new TestThreadSafety();

        Thread testingReferenceThread = new Thread(new Runnable() {

            @Override
            public void run() {
                long countOfIterations = 0L;
                while(true){
                    boolean flag = instance.a != instance.a;
                    if(flag)
                        System.out.println(countOfIterations + ":" + flag);

                    countOfIterations++;
                }
            }
        });

        Thread updatingReferenceThread = new Thread(new Runnable() {

            @Override
            public void run() {
                while(true){
                    instance.a = new Object();
                }
            }
        });

        testingReferenceThread.start();
        updatingReferenceThread.start();
    }

}

This is the program that I am using to test the thread-safety.

Weird behavior

As my program starts between some iterations I get the output flag value, which means that the reference != check fails on the same reference. BUT after some iterations the output becomes constant value false and then executing the program for a long long time does not generate a single true output.

As the output suggests after some n (not fixed) iterations the output seems to be constant value and does not change.

Output:

For some iterations:

1494:true
1495:true
1496:true
19970:true
19972:true
19974:true
//after this there is not a single instance when the condition becomes true
8
What do you mean by "thread-safe" in this context? Are you asking if it's guaranteed to always return false?JB Nizet
@JBNizet yes. That's what I was thinking of.Narendra Pathai
It doesn't even always return false in a single-threaded context. It could be a NaN..harold
Probable explanation: the code was just-in-time compiled and the compiled code loads the variable only once. This is expected.Marko Topolnik
Printing individual results is a poor way to test for races. Printing (both formatting and writing the results) is relatively costly compared to your test (and sometimes your program will end up blocking on write when the bandwith of the connection to the terminal or the terminal itself is slow). Also, IO often contains mutexes of its own which will permute the execution order of your threads (note your individual lines of 1234:true never smash each other). A race test needs a tighter inner loop. Print a summary at the end (as someone did below with a unit test framework).Ben Jackson

8 Answers

124
votes

In the absence of synchronization this code

Object a;

public boolean test() {
    return a != a;
}

may produce true. This is the bytecode for test()

    ALOAD 0
    GETFIELD test/Test1.a : Ljava/lang/Object;
    ALOAD 0
    GETFIELD test/Test1.a : Ljava/lang/Object;
    IF_ACMPEQ L1
...

as we can see it loads field a to local vars twice, it's a non-atomic operation, if a was changed in between by another thread comparison may produce false.

Also, memory visibility problem is relevant here, there is no guarantee that changes to a made by another thread will be visible to the current thread.

47
votes

Is the check a != a thread-safe?

If a can potentially be updated by another thread (without proper synchronization!), then No.

I tried to program this and use multiple threads but didn't fail. I guess could not simulate race on my machine.

That doesn't mean anything! The issue is that if an execution in which a is updated by another thread is allowed by the JLS, then the code is not thread-safe. The fact that you cannot cause the race condition to happen with a particular test-case on a particular machine and a particular Java implementation, does not preclude it from happening in other circumstances.

Does this mean that a != a could return true.

Yes, in theory, under certain circumstances.

Alternatively, a != a could return false even though a was changing simultaneously.


Concerning the "weird behaviour":

As my program starts between some iterations I get the output flag value, which means that the reference != check fails on the same reference. BUT after some iterations the output becomes constant value false and then executing the program for a long long time does not generate a single true output.

This "weird" behaviour is consistent with the following execution scenario:

  1. The program is loaded and the JVM starts interpreting the bytecodes. Since (as we have seen from the javap output) the bytecode does two loads, you (apparently) see the results of the race condition, occasionally.

  2. After a time, the code is compiled by the JIT compiler. The JIT optimizer notices that there are two loads of the same memory slot (a) close together, and optimizes the second one away. (In fact, there's a chance that it optimizes the test away entirely ...)

  3. Now the race condition no longer manifests, because there are no longer two loads.

Note that this is all consistent with what the JLS allows an implementation of Java to do.


@kriss commented thus:

This looks like this could be what C or C++ programmers calls "Undefined Behavior" (implementation dependent). Seems like there could be a few UB in java in corner cases like this one.

The Java Memory Model (specified in JLS 17.4) specifies a set of preconditions under which one thread is guaranteed to see memory values written by another thread. If one thread attempts to read a variable written by another one, and those preconditions are not satisfied, then there can be a number of possible executions ... some of which are likely to be incorrect (from the perspective of the application's requirements). In other words, the set of possible behaviours (i.e. the set of "well-formed executions") is defined, but we can't say which of those behaviours will occur.

The compiler is allowed to combine and reorder loads and save (and do other things) provided the end effect of the code is the same:

  • when executed by a single thread, and
  • when executed by different threads that synchronize correctly (as per the Memory Model).

But if the code doesn't synchronize properly (and therefore the "happens before" relationships don't sufficiently constrain the set of well-formed executions) the compiler is allowed to reorder loads and stores in ways that would give "incorrect" results. (But that's really just saying that the program is incorrect.)

27
votes

Proved with test-ng:

public class MyTest {

  private static Integer count=1;

  @Test(threadPoolSize = 1000, invocationCount=10000)
  public void test(){
    count = new Integer(new Random().nextInt());
    Assert.assertFalse(count != count);
  }

}

I have 2 fails on 10 000 invocations. So NO, it is NOT thread safe

15
votes

No, it is not. For a compare the Java VM must put the two values to compare on the stack and run the compare instruction (which one depends on the type of "a").

The Java VM may:

  1. Read "a" two times, put each one on the stack and then and compare the results
  2. Read "a" only one time, put it on the stack, duplicate it ("dup" instruction) and the run the compare
  3. Eliminate the expression completely and replace it with false

In the 1st case, another thread could modify the value for "a" between the two reads.

Which strategy is chosen depends on the Java compiler and the Java Runtime (especially the JIT compiler). It may even change during the runtime of your program.

If you want to make sure how the variable is accessed, you must make it volatile (a so called "half memory barrier") or add a full memory barrier (synchronized). You can also use some hgiher level API (e.g. AtomicInteger as mentioned by Juned Ahasan).

For details about thread safety, read JSR 133 (Java Memory Model).

6
votes

It has all been well explained by Stephen C. For fun, you could try to run the same code with the following JVM parameters:

-XX:InlineSmallCode=0

This should prevent the optimisation done by the JIT (it does on hotspot 7 server) and you will see true forever (I stopped at 2,000,000 but I suppose it continues after that).

For information, below is the JIT'ed code. To be honest, I don't read assembly fluently enough to know if the test is actually done or where the two loads come from. (line 26 is the test flag = a != a and line 31 is the closing brace of the while(true)).

  # {method} 'run' '()V' in 'javaapplication27/TestThreadSafety$1'
  0x00000000027dcc80: int3   
  0x00000000027dcc81: data32 data32 nop WORD PTR [rax+rax*1+0x0]
  0x00000000027dcc8c: data32 data32 xchg ax,ax
  0x00000000027dcc90: mov    DWORD PTR [rsp-0x6000],eax
  0x00000000027dcc97: push   rbp
  0x00000000027dcc98: sub    rsp,0x40
  0x00000000027dcc9c: mov    rbx,QWORD PTR [rdx+0x8]
  0x00000000027dcca0: mov    rbp,QWORD PTR [rdx+0x18]
  0x00000000027dcca4: mov    rcx,rdx
  0x00000000027dcca7: movabs r10,0x6e1a7680
  0x00000000027dccb1: call   r10
  0x00000000027dccb4: test   rbp,rbp
  0x00000000027dccb7: je     0x00000000027dccdd
  0x00000000027dccb9: mov    r10d,DWORD PTR [rbp+0x8]
  0x00000000027dccbd: cmp    r10d,0xefc158f4    ;   {oop('javaapplication27/TestThreadSafety$1')}
  0x00000000027dccc4: jne    0x00000000027dccf1
  0x00000000027dccc6: test   rbp,rbp
  0x00000000027dccc9: je     0x00000000027dcce1
  0x00000000027dcccb: cmp    r12d,DWORD PTR [rbp+0xc]
  0x00000000027dcccf: je     0x00000000027dcce1  ;*goto
                                                ; - javaapplication27.TestThreadSafety$1::run@62 (line 31)
  0x00000000027dccd1: add    rbx,0x1            ; OopMap{rbp=Oop off=85}
                                                ;*goto
                                                ; - javaapplication27.TestThreadSafety$1::run@62 (line 31)
  0x00000000027dccd5: test   DWORD PTR [rip+0xfffffffffdb53325],eax        # 0x0000000000330000
                                                ;*goto
                                                ; - javaapplication27.TestThreadSafety$1::run@62 (line 31)
                                                ;   {poll}
  0x00000000027dccdb: jmp    0x00000000027dccd1
  0x00000000027dccdd: xor    ebp,ebp
  0x00000000027dccdf: jmp    0x00000000027dccc6
  0x00000000027dcce1: mov    edx,0xffffff86
  0x00000000027dcce6: mov    QWORD PTR [rsp+0x20],rbx
  0x00000000027dcceb: call   0x00000000027a90a0  ; OopMap{rbp=Oop off=112}
                                                ;*aload_0
                                                ; - javaapplication27.TestThreadSafety$1::run@2 (line 26)
                                                ;   {runtime_call}
  0x00000000027dccf0: int3   
  0x00000000027dccf1: mov    edx,0xffffffad
  0x00000000027dccf6: mov    QWORD PTR [rsp+0x20],rbx
  0x00000000027dccfb: call   0x00000000027a90a0  ; OopMap{rbp=Oop off=128}
                                                ;*aload_0
                                                ; - javaapplication27.TestThreadSafety$1::run@2 (line 26)
                                                ;   {runtime_call}
  0x00000000027dcd00: int3                      ;*aload_0
                                                ; - javaapplication27.TestThreadSafety$1::run@2 (line 26)
  0x00000000027dcd01: int3   
5
votes

No, a != a is not thread safe. This expression consists of three parts: load a, load a again, and perform !=. It is possible for another thread to gain the intrinsic lock on a's parent and change the value of a in between the 2 load operations.

Another factor though is whether a is local. If a is local then no other threads should have access to it and therefore should be thread safe.

void method () {
    int a = 0;
    System.out.println(a != a);
}

should also always print false.

Declaring a as volatile would not solve the problem for if a is static or instance. The problem is not that threads have different values of a, but that one thread loads a twice with different values. It may actually make the case less thread-safe.. If a isn't volatile then a may be cached and a change in another thread won't affect the cached value.

3
votes

Regarding the weird behaviour:

Since the variable a is not marked as volatile, at some point it might value of a might be cached by the thread. Both as of a != a are then the cached version and thus always the same (meaning flag is now always false).

0
votes

Even simple read is not atomic. If a is long and not marked as volatile then on 32-bit JVMs long b = a is not thread-safe.