I wanted to ask for some help in solving the data races in my program. I started with a simulation of how things should work if I were using multithreading and then modified the code so that I can check if I really obtain those results but I don't know how to resolve the data races in the code. Can someone help me please? My code is the following:

#include <cstdlib>
#include <iostream>
#include <ctime>
#include <cstring>
#include <cmath>
#include <random>
#include <thread>
#include <algorithm>
#include <functional>

using namespace std;

void pimontecarlo(size_t& threadID, size_t numThreads, size_t N, double& pi)
{
    size_t begin = threadID * N / numThreads;
    size_t end = (threadID   1) * N / numThreads;

    random_device rd;       // Object to create random seed
    mt19937 generator(rd());    // Mersenne Twister seeded with rd()
        uniform_real_distribution<double> distribution(-1.0, 1.0);

        for (size_t i = begin; i < end; i  ) {
                double x = distribution(generator);
                double y = distribution(generator);
                if (sqrt(x*x   y*y) < 1.0)
                        pi  = 4.0 / double(N);
        }
}

int main(int argc, char* argv[])
{
        if (argc != 2) {
                cerr << "Usage: ./pimontecarlo <number of threads>" << endl;
                exit(EXIT_FAILURE);
        }

        size_t numThreads = (size_t)atoi(argv[1]);

        size_t N = 100000000;
        cout << "Generating " << N << " random values using "
             << numThreads << " thread(s)." << endl;

    double pi = 0.0;

        // create threads
        vector<thread> myThreads(numThreads);
        for (size_t i = 0; i < numThreads; i  )
                myThreads[i] = thread(pimontecarlo, ref(i), numThreads, N, ref(pi));

        for_each(myThreads.begin(), myThreads.end(), mem_fn(&thread::join));

        cout.precision(12);
        cout << "Estimated value for pi: " << pi << endl;

        exit(EXIT_SUCCESS);
}

I tried using valgrind in order to find the possible data races and I found out that my variable 'pi' is one and from what I understand it should be resolved by making it an atomic variable. Another data race it says is at the row 66 in the for loop, I think probably because I use 'i' as argument of my function but that I don't know how to resolve.

CodePudding user response：

I don't know how to resolve the data races in the code.

Then stop using std::thread, and especially stop passing references to functions on those threads.

Here is one way you could avoid passing any references between threads:

#include <cstdlib>
#include <iostream>
#include <random>
#include <future>
#include <algorithm>

int pimontecarlo(size_t iterations)
{
    int count = 0;

    std::random_device rd;
    std::mt19937 generator(rd());
    std::uniform_real_distribution<double> distribution(-1.0, 1.0);

    for (size_t i = 0; i < iterations; i  ) {
        double x = distribution(generator);
        double y = distribution(generator);
        if (sqrt(x*x   y*y) < 1.0)   count;
    }

    return count;
}

int main(int argc, char* argv[])
{
    if (argc != 2) {
        std::cerr << "Usage: ./pimontecarlo <number of threads>" << std::endl;
        exit(EXIT_FAILURE);
    }

    size_t numThreads { atoi(argv[1]) };

    size_t N = 100000000;
    std::cout << "Generating " << N << " random values using " << numThreads << " thread(s)." << std::endl;

    // create futures
    std::vector<std::future<int>> results(numThreads);
    for (auto & result : results)
        result = std::async(pimontecarlo, N / numThreads);

    int count = 0;
    for (auto & result : results)
        count  = result.get();

    double pi = (4.0 * count) / N;

    std::cout.precision(12);
    std::cout << "Estimated value for pi: " << pi << std::endl;

    exit(EXIT_SUCCESS);
}

See it on coliru