0

I am currently trying to get a better understanding of multithreading and am experimenting using std::thread.

I have this piece of code:

volatile int m = 0;
void ThdFun(int i)
{
    for (int j = 0; j < i; ++j) {
        m++;
        //std::cout << "hello1 " << m << " " <<std::endl;
    }
    //std::cout << "Hello World from thread " << i <<"  " << std::endl;
}
int main(){
    int var = 100000; //changes when i want to experiment with diff values

    std::thread t1(ThdFun, var);
    std::thread t2(ThdFun, var);
    t1.join();
    t2.join();
    std::cout << "final result of m: " << m << std::endl;

    if ((var * 2) == m) {
        std::cout << "CORRECT" << std::endl;
    }
    else
        std::cout << "INCORRECT" << std::endl;
    return 0;
}

What I noticed is that if my var = 2 or var =50, I get the correct output (which is 4 and 100 respectively)

But when I make var a large number like 100000, I get anything in the range of 100000-200000, when I would expect to get 200000. I'm wondering why this happens, because to my understanding isn't the 'join()' function supposed to make it go in sequential order? Also why is the output fine for 'smaller' numbers but it gets unpredictable with larger numbers? Is 1 thread finishing and thus exiting the program or something?

Could someone please explain what is happening that causes this varying output for large numbers, and why the join() doesn't work well for larger numberss?

Thank you!

273K
  • 29,503
  • 10
  • 41
  • 64
Megan Darcy
  • 530
  • 5
  • 15

1 Answers1

2

Your problem is the m++; line. What it really does, is something like this:

  1. Read m from memory into register
  2. Increment the register value
  3. Write the register value back to m

The problem with your code is that the access to m is not synchronized between the threads, so it is possible for one thread to read m (step 1) and before it gets to write back the incremented value (step 3), the other thread has read the old value of m.

In that case both threads read and increment the value as it was before the other one incremented it, so you lose one increment.

The solution to this is to add synchronization.

This is fortunately very simple, just change the definition of m to std::atomic_int and it'll work. When the variable is declared atomic, the language guarantees that the m++ operation cannot be interrupted by another thread.

An alternative to atomic is to use a mutex, but with just a single access, the use of atomic is preferable.

cptFracassa
  • 893
  • 4
  • 14
  • 1
    Formally, the issue here is that executing `m++` from multiple threads introduces a data race: multiple threads are accessing the same object, and at least one of them is writing to it. The solution is to add synchronization. (`volatile` doesn't do that). +1. – Pete Becker Dec 03 '21 at 14:27
  • @PeteBecker You are of course correct., so Iexpanded the answer a bit, based on your comment. – cptFracassa Dec 03 '21 at 15:53