2

I think my program has a bug because sometimes when I run my program, it outputs a lower number than 30000, such as 29999. But sometimes it runs correctly and gets to the 30000. My question is how can I fix this and why is it happening.

#include <iostream>
#include <thread>

using namespace std;

int counter;
int i;

void increment()
{
counter++;
}

int main()
{
counter = 0;

cout << "The value in counter is : " << counter << endl;
thread tarr[30000];

    for (i = 0; i < 30000; i++)
    {
        tarr[i] = thread(increment);
    }

    for (i = 0; i < 30000; i++)
    {
        tarr[i].join(); //main thread waits for tarr to finish
    }

cout << "After running 30,000 threads ";
cout << "the value in counter is : " << counter << endl;
return 0;
}
Tas
  • 7,023
  • 3
  • 36
  • 51
Benjamin Fiskeaux
  • 17
  • 2

1 Answers1

2

The problem is that counter++ can be broken down into three operations:

  1. Load initial value to register
  2. Increment the value
  3. Store the new value back to memory

A single thread may do the first two steps, then pass up control to another thread to do the same. What this can mean is:

  1. Thread one reads counter as 5
  2. Thread one increments its internal copy to 6
  3. Thread two reads counter as 5
  4. Thread two increments its internal copy to 6
  5. Thread two writes back 6 to counter
  6. Thread one writes back 6 to counter

You should make counter std::atomic, or guard it with a std::mutex:

std::atomic<int> counter;
Tas
  • 7,023
  • 3
  • 36
  • 51
  • While the "broken in three operations" part sounds reasonable, according to C++ it's actually Undefined Behavior. This is important, because the model provided here suggests that the range of possible outcomes is smaller than it is in reality. In particular, this answer suggests that the counter must be incremented by _at least_ one thread, so the outcome must be between 1 and 30.000. A reasonable optimizing compiler may remove all unsafe assignments to `counter`, producing an equally valid outcome of 0. Still, the final conclusion of this answer is correct. – MSalters Apr 19 '17 at 09:30