Puzzled by Java: boolean assignment fails in Thread

Question

I am quite puzzled by certain behavior in Java, and I was wondering if somebody could provide an explanation. I am trying to set a boolean value to true to stop a thread, but assignment fails. Consider the following example:

public class Temp {

public class Unstoppable implements Runnable {
    public boolean stop=false;
    private int ctr=0;
    @Override
    public void run() {
        while(!stop) {
            stop |= doSomething();
        }
    }

    public boolean doSomething() {
        System.out.println("Still running "+ctr++);

        // some other logic here could decide that it's time to stop
        // especially if Unstoppable would be an abstract class and doSomething() an abstract function  
        return false;
    }

    public void stop() {
        stop=true;
    }
}

public void start() {
    // start thread with Unstoppable
    Unstoppable st = new Unstoppable();
    new Thread(st).start();

    // wait for a while
    try {
        Thread.sleep(2000);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }

    // try to stop the thread
    st.stop(); // assignment fails, variable 'stop' is still false after this call so Unstoppable never stops
}

public static void main(String[] args) {
    Temp t = new Temp();
    t.start();
}
}

Trying to assign the value true in the stop() function simply fails and the thread keeps running. I found out that changing the code to below resolves the problem:

@Override
    public void run() {
        while(!stop) {
            // without   stop |=   the thread DOES stop
            doSomething();
        }
    }

but I can't understand why.

More bizarrely, the code change below also resolves the problem:

    @Override
    public void run() {
        while(!stop) {
            stop |= doSomething();

            // printing here does also result in the thread stopping!
            System.out.println("Still running "+ctr++);
        }
    }

    public boolean doSomething() {
        // some other logic here could decide that it's time to stop
        // especially if Unstoppable would be an abstract class and doSomething() an abstract function  
        return false;
    }

Although I can resolve the problem, I'd like to understand what's going on here. Thanks!

Edit Just some more clarification, I changed the code into the following:

public class Temp {

public class Unstoppable implements Runnable {
    private volatile boolean stop=false;

    @Override
    public void run() {
        while(!stop) {
            System.out.println("A) stop="+stop);
            stop |= doSomething();
            System.out.println("C) stop="+stop);
        }
    }

    public boolean doSomething() {
        while(!stop) {
        }
        System.out.println("B) stop="+stop);
        // some other logic here could decide that it's time to stop
        // especially if Unstoppable would be an abstract class and doSomething() an abstract function  
        return false;
    }

    public void setStop(boolean stop) {
        System.out.println("D) stop="+stop);
        this.stop=stop;
        System.out.println("E) stop="+stop);
    }
}

public void start() {
    // start thread with Unstoppable
    Unstoppable st = new Unstoppable();
    Thread t = new Thread(st);
    t.start();

    // wait for a while
    try {
        Thread.sleep(2000);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }

    // try to stop the thread
    st.setStop(true); // assignment fails, variable 'stop' is still false after this call so Unstoppable never stops
}

public static void main(String[] args) {
    Temp t = new Temp();
    t.start();
}
}

This results in the following statements on the console:

A) stop=false
D) stop=true
E) stop=true
B) stop=true
C) stop=false
A) stop=false

The puzzlement was on statement C) stop=false. At B) it was true, the function then results false, and I would expect true |= false to result in true...

However, as slim showed, the left side of the |= was already evaluated by Java before doSomething() was called. Changing the code to :

@Override
public void run() {
    while(!stop) {
        boolean stopNow = doSomething();
        stop |= stopNow;
    }
}

Does result in the thread being stopped.

Answer 1

stop |= foo()

... is an abbreviation of:

boolean x = foo();
boolean y = stop || x;
stop = y;

Now consider two threads:

     Thread A                |  Thread B
1    boolean x = foo();      |
2    boolean y = stop || x;  |  
3                            |  stop = true;
4    stop = y                |
5    if(stop) { ... }

If y is false, then, when things happen in this order, thread B's assignment to stop (3) gets replaced by thread A's assignment (4), before the test (5).

This race condition happens even if stop is volatile, and even if you ignore the "weirdness" of variable visibility between threads.

The point is that stop |= foo() is not atomic, and so stuff can happen during its execution that screws up the apparent logic. This is why we have classes like AtomicBoolean which provide guaranteed atomic operations which you could use for this purpose.

  AtomicBoolean stop = new AtomicBoolean();
  ...
  while(! stop.get()) {
      ...
      stop.compareAndSet(false, foo());
  }

Alternatively you could put the |= into a synchronized method, and make this the only way you ever assign stop:

   private synchronized stopIf(boolean doStop) {
        this.stop |= doStop;
   }