Reputation: 1
I have following code which works in debug build not in the release build with g++ optimizations turned on. (When I say work, what I mean is when the main thread sets the flag stop to true, the looping thread exists).I know this issue can be fixed by adding volatile keyword. My question however is to understand what exactly is happening in this case. This is the code:
void main() {
bool stop = false;
std::thread reader_thread;
auto reader = [&]() {
std::cout << "starting reader" << std::endl;
//BindToCPU(reader_thread, 0);
while(!stop) {
}
std::cout << "exiting reader" << std::endl;
};
reader_thread = std::thread(reader);
sleep(2);
stop = true;
std::cout << "stopped" << std::endl;
reader_thread.join();
}
Why does this happen? Is it because compiler optimizations? Or is it because cache coherency issues? Some details on what exactly happens underneath is appreciated.
Upvotes: 0
Views: 329
Reputation: 4218
You have several threads which access the same variable, and one of them writes to the variable. This situation is called a data race. A data race is undefined behavior, and compilers tend to do funny/catastrophic things in these cases.
One example, which happens to match your description, is stated in here in section 2.3:
... violate the compiler's assumption that ordinary variables do not change without being assigned to ... If the variable is not annotated at all, the loop waiting for another thread to set flag:
while (!flag) {}
could even be transformed to the, now likely infinite, but sequentially equivalent, loop:
tmp = flag; // tmp is local
while (!tmp) {}
Another article about this type of race condition is this one.
Upvotes: 1
Reputation: 76235
The behavior of the program is undefined. The problem is that two threads are both accessing the variable named stop
, and one of them is writing to it. In the C++ standard, that's the definition of a data race, and the result is undefined behavior. To get rid of the data race you have to introduce synchronization in some form. The simplest way to do this is to change the type of stop
from bool
to std::atomic<bool>
. Alternatively, you could use a mutex, but for this particular example, that would be overkill.
Marking stop
as volatile
might make the symptoms go away, but it does not fix the problem.
Upvotes: 3
Reputation: 17016
The problem is that the compiler, specifically the optimization phase cannot tell that the program actually does anything. In particular, it cannot tell that "stop" can ever be anything except false. The best and simplest solution is to make "stop" atomic. Here is the corrected program, with a bonus "sleep" routine at no extra charge.
#include <iostream>
#include <thread>
#include <chrono>
#include <atomic>
inline void std_sleep(long double seconds) noexcept
{
using duration_t = std::chrono::duration<long long, std::nano>;
const auto duration = duration_t(static_cast<long long> (seconds * 1e9));
std::this_thread::sleep_for(duration);
}
int main() {
std::atomic<bool> stop = false;
std::thread reader_thread;
auto reader = [&stop]() {
std::cout << "starting reader" << std::endl;
//BindToCPU(reader_thread, 0);
while(!stop) {
std_sleep(.25);
}
std::cout << "exiting reader" << std::endl;
};
reader_thread = std::thread(reader);
std_sleep(2.0);
stop = true;
std::cout << "stopped" << std::endl;
reader_thread.join();
return 0;
}
Upvotes: 2