0

The function SentMessage always outputs 100, even if the value has changed. I tried to create pointers, pass p1 in different ways, but it always output 100. How can I read p1 in thread?

Hero p1 = { "16x16 knight 4 v3.png", 100, 100, 3 }, p2 = { "16x16 knight 3 v3.png", 400, 100, 3 };
void SentMessage() {
    while (true) {
        cout << p1.positionX << endl;
    }
}
int main() {
    thread th1(SentMessage);
    windowa.WindowOpen(p1, p2);// here i am updating p1 and p2 position
    th1.join();
}
Improve this question
5
  • 1
    A value accessed by more than one thread should be protected/synchronized e.g. by a mutex or atomic to avoid a race condition (=> UB) and force to read the value in each iteration (otherwise the optimizer can optimize it out). Commented Aug 19, 2024 at 17:48
  • every thread lives in its own small house with paper notes on each wall containing the values of each variable, when a variable is an atomic it knows to not write it on the wall but instead go out of its house to read it or write to it, otherwise it will just write it on a note on the wall unaware of whether its neighboring threads have modified it, unless a synchronization point happen like a mutex, in which case it tears all paper notes on the wall and reads new values for all variables, that's pretty much C++'s memory model. Commented Aug 19, 2024 at 17:51
  • Try eliminating the windows.WindowOpen() function call. Add a loop in main that delays, (sleeps) then changes the global variables. You should also disassemble the SendMessage() function to verify the compiler did not optimize-out the reading of the global variable. Commented Aug 19, 2024 at 18:47
  • Is it a data race? Commented Aug 20, 2024 at 3:53
  • The dupe is about whether the case is indeed a data race. It contains no solution to the actual problem but only answers if it is or isn't a race (with quotes from the standard). But here the OP is not even aware of this issue. So that dupe will not be very helpful for them. My answer below explains it and also offers a practial solution. There is also the issue of the optimizer. Therefore I reopened. Commented Aug 20, 2024 at 4:05

1 Answer 1

Reset to default
1

Accessing data from multiple threads here without protection/synchronization causes a race condition which leads to undefined behavior.

In addition the optimizer is permitted to optimize out reading the value of p1.positionX in each iteration (since it is not protected, and does not seem to be modified within the loop by the current thread).

The solution is to use some multithreading synchronization mechanism to protect the access, and also force reading of the value in each iteration.

You didn't post the defintion of the class/struct Hero.
If p1.positionX was a e.g. a simple int variable, it could have been made into a std::atomic<int>.
But in this case it is probably better to use e.g. a std::mutex to protect the access to the whole struct/class.

Below is an example how you might use std::mutex to synchronize access to p1.
Note that although it is not a must, it is very much recommended to use it with a RAII class like std::lock_guard to automatically unlock the mutex at the end of the scope:

  1. Add the mutex at the same scope as p1: 
    #include <mutex>

Hero p1 = { "16x16 knight 4 v3.png", 100, 100, 3 };
std::mutex p1_mutex;
  2. Reading the value in SentMessage: 
    { // start scope for synchronized access
    std::lock_guard<std::mutex> lock(p1_mutex); // will lock the mutex
    cout << p1.positionX << endl;
} // here scope for `lock` will end and it will unlock the mutex
  3. Updating the value in WindowOpen: 
    { // start scope for synchronized access
    std::lock_guard<std::mutex> lock(p1_mutex); // will lock the mutex
    p1.positionX++;
} // here scope for `lock` will end and it will unlock the mutex

A final note:

You didn't show the code for WindowOpen.
Make sure that p1 and p2 are accpeted by reference (i.e. that the parameters are Hero &). Otherwise a copy of them is passed and the original variables will not be updated anyway.

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

6 Comments

windows.Window Open(p1, p2); I have it in the main thread, so the mutex doesn't help, and the atomic doesn't work
The main thread and th1 are 2 separate threads, so you need a mutex or equivalent (the main thread is also a thread).
How i can use mutex with sfml?
This is not specific to sfml, but anyway I updated my answer with an example. Also see the final note about accepting the objects by reference.
I looked at your updated comment, did as you wrote and it worked, thank you very much
|

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.