What is the difference between passing a loop counter or range-declaration to a thread by reference?

Question

#include <iostream>
#include <memory>
#include <vector>

#include "boost/thread.hpp"

using boost::thread;
using std::vector;
using std::unique_ptr;

class ThreadPool {
 public:
  ThreadPool(int size) : max_size_(size) {}
  ~ThreadPool() {
    Join();
  }
  void PollForSpace() {
    while (static_cast<int>(pool_.size()) >= max_size_) {
      for (auto it = pool_.begin(); it != pool_.end(); ++it) {
        if ((*it)->timed_join(boost::posix_time::milliseconds(10))) {
          pool_.erase(it);
          break;
        }
      }
    }
  }

  void AddToPool(unique_ptr<thread> t) {
    pool_.push_back(std::move(t));
  }

  void Join() {
    for (auto it = pool_.begin(); it != pool_.end(); ++it) {
      (*it)->join();
    }
    pool_.clear();
  }
 protected:
  vector<unique_ptr<thread> > pool_;
  int max_size_;
};

int main(int argc, char** argv) {
  ThreadPool pool(20);

  std::vector<int> integers;
  for (int i = 0; i < 100; ++i) {
    integers.push_back(i);
  }

  std::cout << "Range based for loop over vector." << std::endl;

  for (auto const& i : integers) {
    pool.PollForSpace();
    pool.AddToPool(unique_ptr<thread>(new thread([&i]{
          std::cout << i << std::endl;
          })));
  }
  pool.Join();

  std::cout << "Integer loop." << std::endl;

  for (int i = 0; i < 100; ++i) {
    pool.PollForSpace();
    pool.AddToPool(unique_ptr<thread>(new thread([&i]{
          std::cout << i << std::endl;
          })));
  }
  pool.Join();  

  return 0;
}

Why does the range-based for-loop properly print out the numbers 0-99 (although not necessarily in order, and with occasionally newlines interspersed incorrectly), but the integer loop causes print out like this:

1
2
3
4
5
6
7
8
9
13
1414

14
14
15
16
18
18
19
...
99
100

As I understand it, the integer is being passed by reference to the thread, but its value is changed in the main loop before the thread gets to print it out, causing some values to be not printed out, some values to be printed out more than once, and the value 100 to be printed, even though i obtains that value after the last thread is created.

However, I don't see why this is also not an issue when using the range-based for-loop.

Why does the range-based for-loop version not suffer from the same concurrency issues as the plain integer loop in this code?

Answer 1

for (int i = 0; i < 100; ++i) {
    pool.PollForSpace();
    pool.AddToPool(unique_ptr<thread>(new thread([&i]{
        std::cout << i << std::endl;
      })));
}

You're capturing variable i to the lambda by reference. All the threads now have a reference to the same instance of i, which you're changing in the loop. You've already mentioned you're aware of this happening

Your range based approach has a vector with discrete instances of i so they aren't changing in the loop.

It effectively unrolls to this:

for (auto itr = begin(integers); itr != end(integers); ++itr)
{
    int const& i = *itr;
    // pass new reference to each thread
    pool.PollForSpace();
    pool.AddToPool(unique_ptr<thread>(new thread([&i]{
        std::cout << i << std::endl;
      })));
}

You can't reseat a reference in C++