Effect of compiler optimization in C++

Question

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.

Try making `stop` [atomic](http://en.cppreference.com/w/cpp/atomic/atomic). But you really shouldn't use a tight loop like that. — Fred Larson, Mar 29 '18 at 20:44
your `while` loop does nothing and compiler just optimizes it away. Making `stop` volatile tells compiler that this is an important var and it could be changed from outside of this context. it will keep the loop in this case. — Serge, Mar 29 '18 at 20:46
@Serge, `volatile` is never right except when dealing with memory-mapped hardware or unix signal handlers. Never. — Jive Dadson, Mar 29 '18 at 21:20
In modern C++ an infinite loop that does nothing has undefined behaviour. With optimisation on, your compiler is probably removing it completely. You need to ensure the loop has SOME visible effect that the compiler can detect. — Peter, Mar 29 '18 at 21:23
For all those who commented above, My question was what happens underneath! I know how to fix this already.. I just dont want to blindly fix it.. — Pete, Apr 01 '18 at 14:38

Pete Becker · Answer 1 · 2018-03-29T22:20:19.927

3

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.

edited Mar 29 '18 at 22:20

answered Mar 29 '18 at 21:07

Pete Becker

74,985
8
76
165

Shhhh! Don't mention `volatile`. – Jive Dadson Mar 29 '18 at 21:12
I don't think the problem is a data race. There's no ABA going on for example, and no problem with memory order. I think it's a problem with the bool getting optimized away. In any case, the fix is to make it `std::atomic` – Jive Dadson Mar 29 '18 at 21:15
Probably the compiler is optimizing away the whole loop in main. Again, `std::atomic`. (And ixnay on the olatilevey.) – Jive Dadson Mar 29 '18 at 21:22
@JiveDadson -- the language definition says that when two or more threads access the same data without synchronization and at least one of those threads is modifying the data you have a data race. That's what this code does. Adding appropriate synchronization removes the data race. – Pete Becker Mar 29 '18 at 21:31
@PeterBecker - Yes, there is a data race, but if that were the only problem the program would still work, because it does not matter who wins the race. – Jive Dadson Mar 29 '18 at 21:34
I am curious why you say that volatile would not actually fix this problem, as I understand it volatile (at least applied to primatives) is an unoptimized access requirement. std::atomic may be a more modern method of acheiving the same but that does not mean that older methods fail. And Andrei Alexandrescu has the following using volatile to fix exactly this issue: http://www.drdobbs.com/cpp/volatile-the-multithreaded-programmers-b/184403766 – SoronelHaetir Mar 29 '18 at 21:35
@SoronelHaetir, All that `volatile` means is that the compiler is to treat every read or write to it as an observable side-effect. While that might (would?) be sufficient in this case, it is certainly not something to recommend. Sometimes data-races matter. – Jive Dadson Mar 29 '18 at 21:39
@SoronelHaetir, At the risk of belaboring the point, `volatile` was never intended for inter-thread communication. Both threads and atomic were introduced in C++11. That Dr. Dobbs article was written in 2001. Back then, there was no native thread support, and programmers (such as myself) just had to make do with what they could scrounge. – Jive Dadson Mar 29 '18 at 22:03
@JiveDadson -- re: "it doesn't matter who wins the race" -- I'm getting tired of saying this. A data race, according to the language definition, means that the behavior of the program is undefined. Full stop. You seem to be applying a colloquial notion of "data race" which does not apply here. – Pete Becker Mar 29 '18 at 22:07
1

@SoronelHaetir -- `volatile` might address part of the problem, but it's not sufficient. See [this answer](https://stackoverflow.com/questions/14624776/can-a-bool-read-write-operation-be-not-atomic-on-x86/14625122#14625122) for more details. – Pete Becker Mar 29 '18 at 22:21

Jive Dadson · Answer 2 · 2018-03-29T22:07:06.697

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;
}

score 1 · Answer 3 · answered Mar 29 '18 at 22:26

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.

Effect of compiler optimization in C++

3 Answers3