c++multithreadingsfmlwinsock

The function in the thread does not output the value correctly


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

Solution

  • 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.