Is accessing while loop bool from other thread, thread safe?

Question

I am writing some java code and have a contrived example, and am not really sure if this is thread safe code.

Basically I have a while loop that checks a bool, and I want to set that bool false from another thread.

Observe the following

    public void start() {
    mRunning = true;

    thread = new Thread()
    {
        public void run()
        {
            while (mRunning) {

            }
        }
    }
    thread.start();
}

Then I have a mutator on the class that can be called from another thread...

public void stop() {
    mRunning = false;
}

Could this code result in undesirable behaviour? If so, should I just just do this?

public void stop() {
    sychronized(this) {
        mRunning = false;
    }
}

The JVM can arbitrarily make copies of instance variables from one thread to another, thus your data in that `boolean` might be stale, to prevent the JVM from doing that, use the` volatile` modifier. `synchronized` achieves the same thing but I guess it's worse because you lock on an instance of your class. Although Oleksandr nailed it when be said that this should be preferred when only one thread is the writer. — Coder-Man, Jun 23 '18 at 21:03
@Wow, `synchronized` is not guaranteed to "achieve the same thing," unless both the writer _and the reader_ synchronize on the _same lock object_ when accessing the variable. — Solomon Slow, Jun 24 '18 at 17:35
@jameslarge this is what I meant. `you lock on an instance of your class` — Coder-Man, Jun 24 '18 at 20:04

Oleksandr Pyrohov · Accepted Answer · 2018-06-24T16:00:55.523

If you need only inter-thread communication, and not mutual exclusion, use the volatile variable:

private static volatile boolean mRunning = false;

If only one unique thread modifies a variable, and other threads only read this variable, then you do not need additional synchronization, volatile will suffice (it guarantees that any thread that reads the field will see the most recently written value).

Should I just do this?

public void stop() {
    synchronized (this) {
        mRunning = false;
    }
}

You can, but you should not! Anyway, you will get unexpected behavior (this answer explains why).

In the situation described in the question, just use a volatile variable, that is also less verbose and whose performance is likely to be better in comparison with the synchronized block.

davidxxx · Answer 2 · 2018-06-23T20:20:38.873

Could this code result in undesirable behaviour?

It could.
The thread in stop() will be able to set to false the value of the shared boolean mRunning field.
But the thread in start() may not see the change if this field is not declared as volatile as the thread executing start() doesn't take the monitor on the current object and so the thread memory state may differ from the main memory state.

So start() could go on looping in the while statement.

So you have to ensure that the shared flag is declared as volatile boolean mRunning.

Is accessing while loop bool from other thread, thread safe?

2 Answers2