Hello, I'm starting to deal with multithreading once again ... I found a simple example for clarity of access out of sync, but I don’t understand some things:

struct Counter { int value; Counter() : value(0){} void increment(){ ++value; } }; int main() { Counter counter; std::vector<std::thread> threads; for(int i = 0; i < 5; ++i){ threads.push_back(std::thread([&counter](){ for(int i = 0; i < 100; ++i){ counter.increment(); } })); } for(auto& thread : threads){ thread.join(); } std::cout << counter.value << std::endl; return 0; }

I absolutely can’t understand why different values ​​fly out ... we have join () ie we wait for each of the five threads to execute one after the other (that is, increment 100 times each), I understand that if detach () stood, it would be tin ... and so each of the five streams one after another after all go ... why values ​​other than 500 are obtained. either I don’t take something into account ... isn't it true that there will be 100 in the first stream, and there will be 100 more in the next?

  • You do not take into account that access to the value without synchronization from different streams is UB. A stream in the absence of synchronization has the right to arbitrarily cache the code, transform, unroll cycles and so on. If you think that writing ++value really increases a variable in some memory area, I have bad news for you. - VladD
  • that is, I understand correctly that there will be no problem when only one thread is added? threads.push_back (std :: thread ([& counter] () {for (int i = 0; i <100; ++ i) {counter.increment ();}})); threads.at (0) .join (); - xperious
  • Guaranteed problems with only one thread. Sidestream = possible (but not required) problems. - VladD
  • I did not understand something ... we have a second thread that increments without mutexes, etc. some value ... is this ub going? - xperious
  • That's right, more than one thread = bad. stackoverflow.com/a/28591924/276994 - VladD

2 answers 2

You have not synchronized access to the counter , therefore, by virtue of data races, as VladD has correctly pointed out to you, you have in the UB code. To fix the code, you need to synchronize access to the counter inside, or outside. You can do this:

 struct Counter { std::atomic_int value; ... };

Or so:

 std::mutex guard; std::vector<std::thread> threads; for(int i = 0; i < 5; ++i) { threads.push_back(std::thread([&counter, &guard]() { for(int i = 0; i < 100; ++i) { std::lock_guard<std::mutex> lock{guard}; counter.increment(); } })); }

Or even a lot of other options - the main thing is to disappear race (data races)

    Your main mistake here is:

    because we have join () we wait for each of the five threads to execute one by one

    By no means. They can be executed as you like. You expect not to complete, but to complete . Roughly speaking, it is possible that the first thread will be executed last, so that the remaining join 's will be to the already-completed threads. This cycle is just a guarantee that all threads will work, and in what order they will work and how - your join 's have no effect on this.