Why could this code fail?

Question

While reviewing this question I noticed this code:

class MyThread extends Thread {
  private boolean stop = false;

  public void run() {
    while(!stop) {
      doSomeWork();
    }
  }

  public void setStop() {
    this.stop = true;
  }
}

However I don't understand why would this fail. Do other threads not get access to the "actual" stop variable?

How you are calling `setStop()` ? from same instance of MyThread or different ? — Adil Shaikh, May 02 '13 at 19:18
I don't know, unfortunately this is an abstract example from the question I linked to — Jane Panda, May 02 '13 at 19:25
setStop would be getting called from a different thread. all the run method has to do to stop is just return, it wouldn't need a flag. — Nathan Hughes, May 02 '13 at 19:53

score 6 · Answer 1 · answered May 02 '13 at 19:28

The JIT compiler can re-order reads and writes in an application so long as

the actions are sequentially consistent and
the altered actions do not violate intra-thread semantics.

That is just a fancy way of saying, all actions should appear to happen the same way as if it were executed by only a single thread. So you can get the JIT to recompile your code to look like this

class MyThread extends Thread {
  private boolean stop = false;

  public void run() {
    if(!stop){
       while(true){

       }
    }
  }

This is a legal optimization called hoisting. It still acts the same as if serial but offers surprising results when using multiple threads.

By declaring a field volatile you are telling Java not to execute any re orderings. Along with the memory consistency as mentioned by Nathan Hughes

Will the JVM check to see if `doSomeWork()` ever sets the `stop` field before hoisting? I assume yes since otherwise that optimization seems not so optimal :) — dlev, May 02 '13 at 19:33
@dlev Yes, absolutely! That would result in a horrible optimization otherwise. — John Vint, May 02 '13 at 19:35

score 5 · Accepted Answer · edited May 23 '17 at 10:30

5

The instance variable stop needs to be volatile, otherwise there's no guarantee the other threads will see changes to it. There are a lot of conflicting interests at work: threads want a consistent view of the program state, CPUs want to be able to cache data, the JVM wants to be able to reorder instructions. Making the instance variable volatile means that it can't be cached and that happens-before relationships are established that limit instruction reordering.

See this other answer (+1) for a good example of what reordering may happen without marking the variable volatile.

(By the way using interruption for thread cancellation is preferable to using an instance variable.)

edited May 23 '17 at 10:30

Community

1
1

answered May 02 '13 at 19:18

Nathan Hughes

94,330
19
181
276

You should mention hoisting which is the #1 reason this can fail. – John Vint May 02 '13 at 19:23
1

@user2246674 I created my own answer to address hoisting. – John Vint May 02 '13 at 19:28
@John: is that different from instruction reordering? – Nathan Hughes May 02 '13 at 19:31
@NathanHughes You're right it is. I guess it wasn't obvious you were including that specific type of reordering. It seemed your reasoning was more set on memory visibility and not reordering, though you did mention it. – John Vint May 02 '13 at 19:37

Stelios Savva · Answer 3 · 2013-05-02T19:26:34.187

1

The variable stop must be declared as volatile.

Although i prefer using interrupt to stop a thread.

edited May 02 '13 at 19:26

answered May 02 '13 at 19:19

Stelios Savva

812
1
10
30

score 0 · Answer 4 · answered May 02 '13 at 19:19

0

Other threads are not guaranteed to see updated values of stop - you need to establish a "happens before" relationship. The simplest way would be to make stop volatile.

answered May 02 '13 at 19:19

henry

5,923
29
46

Why could this code fail?

4 Answers4