C++ Thread does not detect global variable change

Question

I have 2 threads monitoring the same global state, if the state.shutdown becomes false, the thread run() should return. The code is below.

#include <iostream>
#include <chrono>
#include <thread>
#include <mutex>

using namespace std;

struct State {
    bool shutdown = false;

    ~State() {
        shutdown = true;
    }
};

State state;

#define CHECK_SHUTDOWN     \
  {                        \
    std::cout << (state.shutdown ? " SHUTDOWN " : " NOSHUT ") << typeid(*this).name() << std::endl; \
    if (state.shutdown) {  \
        return;            \
    }                      \
  }

class Mythread {
public:
    void join();
    void run();
    void launch();
    std::thread self_thread;
};

void Mythread::run() {
    while(1) {
        CHECK_SHUTDOWN
    }
}

void Mythread::join() {
    if (self_thread.joinable()) {
        self_thread.join();
    }
}

void Mythread::launch() {
    self_thread = std::thread(&Mythread::run, this);
}

std::mutex mtx;
void shut() {
    std::lock_guard<std::mutex> lock(mtx);   
    state.shutdown = true;
}

int main()
{
    Mythread thread1;
    Mythread thread2;

    thread1.launch();
    thread2.launch();

    std::this_thread::sleep_for(std::chrono::milliseconds(1000));


    //state.shutdown = true;
    shut(); //This makes no difference with the line above

    std::this_thread::sleep_for(std::chrono::milliseconds(100));

    thread1.join();
    thread2.join();


    return 0;
}

However, even I manually set the state.shutdown to be true, the threads can never detect it. I got prints like:

 NOSHUT 8Mythread                                                                                                
 NOSHUT 8Mythread                                                                                                
 NOSHUT 8Mythread                                                                                                                                                                                                                

...Program finished with exit code 0                                                                             
Press ENTER to exit console.

at the end. I'm also confused given that the run() function is never returned, the threads join should hang. However the threads can join successfully.

Any help would be very appreciated here!

Welcome to the wonderful topic of "sequencing", which is a set of rules that specifies how threads "see" changes to objects from other threads. This requires specific, explicit action to be taken by ***both*** threads, the thread that changed an object, and a thread that "sees" the object. The rules are complex, and cannot be fully summarized in one or two paragraphs on stackoverflow.com, unless greatly oversimplified and leaving out a bunch of details. If you really want to learn this subject properly, see your C++ textbook's lengthy discussion of the subject matter. — Sam Varshavchik, May 29 '20 at 02:44
The simple fix would be to make `State::shutdown` an [`std::atomic`](https://en.cppreference.com/w/cpp/atomic/atomic) but you would be well-served by following Sam's advice and reading up on _why_ this is necessary. — cdhowie, May 29 '20 at 02:46
@SamVarshavchik Thanks for your reply. Could you kindly point to me any online resources? I could not find it in my C++ primer book(4th edition). — user3391299, May 29 '20 at 03:41
You need [a more advanced C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — Sam Varshavchik, May 29 '20 at 10:59

score 2 · Accepted Answer · answered May 29 '20 at 09:23

You have a data race on shutdown.

When an evaluation of an expression writes to a memory location and another evaluation reads or modifies the same memory location, the expressions are said to conflict. A program that has two conflicting evaluations has a data race [...]

In shut() you set the shutdown flag using a mutex, but the check is performed without the mutex (and the State destructor doesn't use a mutex either). Thus you have conflicting operations (read + write) on a non-atomic variable, without the proper happens before relation. This is a data race which results in undefined behavior.

The simple solution would be to make shutdown an std::atomic<bool>, then you wouldn't even need the mutex to set the flag.

For more details about data races and the C++ memory model I can recommend this paper which I have co-authored: Memory Models for C/C++ Programmers

C++ Thread does not detect global variable change

1 Answers1