5

I have a cross platform c++ program where I'm using the boost libraries to create an asynchronous timer.
I have a global variable:

bool receivedInput = false;

One thread waits for and processes input

string argStr;
while (1) 
{
     getline(cin, argStr);
     processArguments(argStr);
     receivedInput = true;
}

The other thread runs a timer where a callback gets called every 10 seconds. In that callback, I check to see if I've received a message 

if (receivedInput)
{
    //set up timer to fire again in 10 seconds
    receivedInput = false;
}
else
    exit(1);

So is this safe? For the read in thread 2, I think it wouldn't matter since the condition will evaluate to either true or false. But I'm unsure what would happen if both threads try to set receivedInput at the same time. I also made my timer 3x longer than the period I expect to receive input so I'm not worried about a race condition.

Edit: To solve this I used boost::unique_lock when I set receivedInput and boost::shared_lock when I read receivedInput. I used an example from here

Community
  • 1
  • 1
Reid
  • 99
  • 1
  • 9
  • 3
    ¤ if you wonder if it's safe, then it isn't. no matter if something is technically safe or not, with threading the situation is such that if you don't understand it, then it will in short time develop bugs even if it is perfect to begin with. as it happens the code is tecnically unsafe but practically safe. for example, technically the "checking" thread could become suspended at the `if` while the "input" thread processed seven lines. in practice that ain't gonna happen, though, but with the code like that Bad Things™ *will* happen. :-) Cheers & hth. – Cheers and hth. - Alf Dec 07 '11 at 01:06

3 Answers3

5

This is fundamentally unsafe. After thread 1 has written true to receivedInput it isn't guaranteed that thread 2 will see the new value. For example, the compiler may optimize your code making certain assumptions about the value of receivedInput at the time it is used as the if condition or caching it in a register, so you are not guaranteed that main memory will actually be read at the time the if condition is evaluated. Also, both compiler and CPU may change the order of reads and writes for optimization, for example true may be written to receivedInput before getLine() and processArguments().

Moreover, relying on timing for synchronization is a very bad idea since often you have no guarantees as to the amount of CPU time each thread will get in a given time interval or whether it will be scheduled in a given time interval at all.

A common mistake is to think that making receivedInput volatile may help here. In fact, volatile guarantees that values are actually read/written to the main memory (instead of for example being cached in a register) and that reads and writes of the variable are ordered with respect to each other. However, it does not guarantee that the reads and writes of the volatile variable are ordered with respect to other instructions.

You need memory barriers or a proper synchronization mechanism for this to work as you expect.

Adam Zalcman
  • 26,643
  • 4
  • 71
  • 92
1

You would have to check your threading standard. Assuming we're talking about POSIX threads, this is explicitly undefined behavior -- an object may not be accessed by one thread while another thread is or might be modifying it. Anything can happen.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
0

If your threads use the value of receivedInput to control independent code blocks, but not to synchronize with each other, there is one simple solution:

add "volatile" before receivedInput, so the compiler will not do the optimization preventing the threads share the value of receivedInput.

xiesusu
  • 47
  • 4
  • 3
    I don't want to -1 you because you're new and I don't want you to leave, but I don't want the misconception that `volatile` is useful for multithreading to spread. It isn't useful, `volatile` has nothing to do with threading nor as a method of synchronization. – GManNickG Dec 07 '11 at 07:02
  • Thanks for your comment. Yes, I made a mistake here, the two threads assign new value to the same variable. Synchronization is needed here. But volatile is definitely related to multi-threading. "Generally speaking, the volatile keyword is intended to prevent the compiler from applying any optimizations on the code that assume values of variables cannot change "on their own." (I re-check it from wikipedia) :) – xiesusu Dec 07 '11 at 09:01
  • 1
    Note, though, that multi-threading entails much more than disabling optimizations on a variable. – GManNickG Dec 07 '11 at 09:17