Calling std::shuffle from multiple threads

Question

I'm trying to shuffle copy of vector in multiple threads. My code:

int main(int argc, const char *argv[]) {
    std::vector<int> rw{1, 0, 1, 1, 1, 0, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1};
    std::mutex m;
    auto multithread_shuffle = [rw, &m]() {
        unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
        std::shuffle (rw.begin(), rw.end(), std::default_random_engine(seed));
        std::lock_guard lock(m);
        for (size_t i = 0; i < rw.size(); i++) {
            std::cout << "rw[" << i << "] = " << rw[i] << std::endl;
        }
    };
    std::thread t1(multithread_shuffle);
    std::thread t2(multithread_shuffle);
    t1.join();
    t2.join();
    return 0;
}

But unfortunately, I'm getting a lot of warnings and this two errors:

error: no matching function for call to 'swap(const int&, const int&)'
   swap(*__a, *__b);
   ~~~~^~~~~~~~~~~~
error: no type named 'type' in 'struct std::enable_if<false, void>'

I will be grateful for any advice.

This is all going to happen so fast that odds are really good that both threads will get the same seed and perform identical shuffles. — user4581301, May 24 '18 at 19:30

Xirema · Accepted Answer · 2018-05-24T19:31:48.007

The problem is that the vector becomes const when copied into the lambda's capture parameters. Consider:

int main() {
    int x = 15;
    auto func = [x]{//Capture by value
        x = 20;//Does not compile!
    };
}

For all intents and purposes, x is a const int, not an int.

You can get the same problem in your code with only a minor change:

int main(int argc, const char *argv[]) {
    std::vector<int> rw{1, 0, 1, 1, 1, 0, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1};
    std::mutex m;
    auto multithread_shuffle = [rw, &m]() {
        rw[0] = 3; //Does not compile!
        rw = std::vector<int>{4,3,2,1}; //Does not compile!
        unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
        std::shuffle (rw.begin(), rw.end(), std::default_random_engine(seed));
        std::lock_guard lock(m);
        for (size_t i = 0; i < rw.size(); i++) {
            std::cout << "rw[" << i << "] = " << rw[i] << std::endl;
        }
    };
    std::thread t1(multithread_shuffle);
    std::thread t2(multithread_shuffle);
    t1.join();
    t2.join();
    return 0;
}

You need to explicitly allocate a local copy of the vector if you want it to be mutable.

int main(int argc, const char *argv[]) {
    std::vector<int> rw{1, 0, 1, 1, 1, 0, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1};
    std::mutex m;
    auto multithread_shuffle = [rw, &m]() {
        auto rw_mutable = rw;
        unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
        std::shuffle (rw_mutable.begin(), rw_mutable.end(), std::default_random_engine(seed));
        std::lock_guard lock(m);
        for (size_t i = 0; i < rw.size(); i++) {
            std::cout << "rw_mutable[" << i << "] = " << rw_mutable[i] << std::endl;
        }
    };
    std::thread t1(multithread_shuffle);
    std::thread t2(multithread_shuffle);
    t1.join();
    t2.join();
    return 0;
}

You could also just declare the lambda mutable:

int main(int argc, const char *argv[]) {
    std::vector<int> rw{1, 0, 1, 1, 1, 0, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1};
    std::mutex m;
    auto multithread_shuffle = [rw, &m]() mutable {
        unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
        std::shuffle (rw.begin(), rw.end(), std::default_random_engine(seed));
        std::lock_guard lock(m);
        for (size_t i = 0; i < rw.size(); i++) {
            std::cout << "rw[" << i << "] = " << rw[i] << std::endl;
        }
    };
    std::thread t1(multithread_shuffle);
    std::thread t2(multithread_shuffle);
    t1.join();
    t2.join();
    return 0;
}

Calling std::shuffle from multiple threads

1 Answers1