Reputation: 792
I was optimizing a c++ code in which I encountered a situation that can be simplified as follows.
Consider this code:
#include <iostream>
#include <thread>
using namespace std;
bool hit = false;
void F()
{
this_thread::sleep_for(chrono::seconds(1));
hit = true;
}
int main()
{
thread t(F);
while (!hit)
;
cout << "finished" << endl;
t.join();
return 0;
}
This basically starts a thread which after a second will change the value of hit
to true
. At the same time the code enters an empty loop which will continue until the hit
's value will become true
. I compiled this with gcc-5.4
using -g
flag and everything was fine. The code will output finished
and ends. But then I compiled it with -O2
flag and this time the code got stuck in the loop infinitely.
Looking at the disassembly, the compiler had generated the following, which is the root cause of the infinite loop:
jmp 0x6ba6f3 ! 0x00000000006ba6f3
OK, so clearly, the compiler has deduced that hit
's value is false
and it will not change in the loop so why not assume that it is an infinite loop without considering that another thread may change its value! And this optimization mode is added in the higher level (-O2
). Since I'm not exactly an optimization flag expert, can anyone tell me which of them is responsible for this result so I can turn it off? And would turning it off have any major performance cost for other pieces of code? I mean, how much this pattern of code is rare?
Upvotes: 6
Views: 570
Reputation: 1689
By declaring hit
as volatile, you're telling the compiler that this variable can be modified by external factors at any time, so the compiler won't assume that its value won't change along your main
function.
As long as there is only one thread writing to the hit
variable, your code should work properly, with no race conditions involved. However, when you're dealing with multiple threads, it's always safer to use synchronization tools, like atomic objects, mutexes and semaphores, as already mentioned in the other answers here.
Upvotes: 1
Reputation: 37599
If you want to read/write hit
from several threads at the same time then you need some kind of synchronization otherwise you'll introduce a race condition. You can either make hit
an std::atomic<bool>
or add a mutex
that needs to be locked when accessing hit
value. If you just want to wait for thread to finish its job than you can leave just thread.join()
(and print "finished" after it) without introducing any additional flags.
Upvotes: 2
Reputation: 180295
This code has Undefined Behavior. You're modifying hit
from one thread and reading it form another, without synchronization.
Optimizing hit
to false
is a valid outcome of Undefined Behavior. You can solve this by making hit
a std::atomic<bool>
. This makes if well-defined, and blocks the optimization.
Upvotes: 6