6

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?

Improve this question
6
  • 1
    use std::atomic<bool>. Commented Oct 27, 2017 at 18:00
  • 2
    What happens if you declare hit as volatile? Commented Oct 27, 2017 at 18:02
  • 2
    @Sinapse: No, actually, it doesn't. It only made the symptoms go away - for now. Trouble is, it's still Undefined Behavior, and if you change one line of code the optimizer may make some new decisions. Commented Oct 27, 2017 at 19:01
  • 1
    @MSalters I understand the problem generally, what volatile solves is the compiler's optimization alone! But since bool (1 byte) would not face tearing in read/write and cache coherence does not matter in the example above (it will only cause to loop some more than it actually needs), we are good to go JUST IN THIS case of course Commented Oct 27, 2017 at 19:16
  • 1
    @Sinapse: The Standard is very clear on this point. It's not "tearing" or "cache coherence", the Standard doesn't talk about that. Two accesses to a single variable like this must be ordered, on penalty of Undefined Behavior. volatile does not affect ordering and does not cure the Undefined Behavior. As far as the Standard is concerned, each thread can get its own volatile bool hit ! Commented Oct 27, 2017 at 20:23

3 Answers 3

Reset to default
6

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Yes this too will solve the problem (I think volatile will do it more efficiently for my case) but OMG!! Just how the compiler knows it when you define the variable as atomic?! I mean volatile make sense since you are adding a qualifier but in case of std::atomic, how compiler will deduce that it may change in another thread??
Guess what! There are volatile members inside the std::atomic<bool> class. :) Try opening the "atomic" file from g++ include folder, you can see them there.
Nice, that was worthy!
@Milack27: Implementation detail.
2

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.

Improve this answer

Comments

1

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.

Improve this answer

2 Comments

volatile fails to provide the necessary ordering.
Yes. As I said, it's always safer to use synchronization tools. Using volatile should work properly in this case.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.