I'm working on a multi-threaded project. I am using CMake to compile. I have one file/function that sets a bool to true every so often
#include <chrono>
void mainloop_click(int *cpm, bool *click, bool *end) {
auto start_time = std::chrono::system_clock::now();
while (!*end) {
*click = false;
while (std::chrono::duration<double>(std::chrono::system_clock::now() - start_time).count() < (60.0 / (double) *cpm));
*click = true;
start_time = std::chrono::system_clock::now();
}
}
My test function that is having problems is
void counter(int *count, bool *click, bool *end) {
while (!*end) {
if (*click) { // TODO Fix Release testing error. This always evaluates to false in release
(*count) ;
while (*click) {}
}
}
}
The basic outline of my test for this is:
- Start mainloop_click in its own thread
- Start counter in its own thread, passing the same click pointer.
- Test if the counter found as many clicks as would be expected for whatever the speed was set to (by
cpm
) after a set period of time.
As far as I can tell, in Debug mode, the compiler actually has the if statement evaluating in the executable, but not when compiled as a Release executable, it automatically evaluates the bool, click
, as false (since that's what it is before the thread starts), and doesn't actually check it, but "knows" it's false. Is there anyway to make the Release not do this? I have added print statements, and know that the third line in the test ( With the // TODO
) is where the problem occurs.
CodePudding user response:
Your code contains several errors which results in Undefined Behavior. I'll highlight two in this answer that could explain the behavior you observe.
A bool
, like most objects, can't be used to communicate between threads without synchronization. You have two threads, one that writes to and one that reads from the same variable, which is a data race. Having a data race means the program has Undefined Behavior. You are not allowed to read from an object that another thread might be simultaneously writing to.
The second error is due to the progress guarantee. This guarantee makes it Undefined Behavior to have an infinite loop with no side effects.
One possible result of this UB is that the compiler can see that once the thread enters counter
it never changes *click
, so it can assume that *click
remains constant for the duration of the execution. bool
is not allowed to be shared between threads without synchronization, so this is a valid assumption on the compiler's part.
The compiler could also see that while (*click) {}
has no side effects, so it could assume *click
is eventually false
as the loop may not be infinite.
Since the compiler can assume *click
is a constant, and *click
must eventually be false
, it can assume *click
is always false
.
The easiest solution to this problem is to use a std::atomic<bool>
instead of a bool
. This wraps a bool
in a way that makes it shareable between threads.
However, it looks like you are using non-synchronized objects for many other inter-thread communication applications. You'll have to address each of these.
Multithreading is very difficult to get right, it isn't something that should be learned by trial and error. Consider getting a book on the topic.