CODEWITHSUNDEEP
c++multithreadingclassmember
Daphne
Daphne

Reputation: 175

C++: confusion about accessing class data members while multithreading

I have the following minimal working example in which I create a number of markov_chain objects in a vector chains and an equal number of thread objects in a vector workers, each of which executes a markov_chain class member function sample on each of the corresponding markov_chain objects. This function takes some integer (99 in the below example) and assigns it to the acceptance public data member of the markov_chain object. I then print the value of acceptance for each object in the vector.

#include <iostream>
#include <thread>
#include <algorithm>
#include <vector>


class markov_chain 
{
public:
    unsigned int length{0}, acceptance{0};

    markov_chain(unsigned int l) {length=l;}
    ~markov_chain() {}

    void sample(int acc);
};

void markov_chain::sample(int acc)
{
    acceptance = acc;
    std::cout << length << ' ' << acceptance << std::endl;
}

int main()  
{
    int number_of_threads{3};
    int number_of_samples{1000};

    std::vector<markov_chain> chains;
    std::vector<std::thread> workers;

    for (int i = 0; i <= number_of_threads; i++) {
        chains.push_back(markov_chain(number_of_samples));
        workers.push_back(std::thread(&markov_chain::sample, chains[i], 99));
    }

    std::for_each(workers.begin(), workers.end(), [](std::thread &t) 
    {
        t.join();
    });

    for (int i = 0; i <= number_of_threads; i++) {
        std::cout << chains[i].length << ' ' << chains[i].acceptance << std::endl;
    }

    return 0;
}

Upon executing, the program outputs

1000 99
1000 99
1000 99
1000 99
1000 0
1000 0
1000 0
1000 0

So the program failed to change the value of acceptance for the objects in the vector chains. I don't know why this happens; the function sample successfully assigns the desired value when I use it without creating threads.

Upvotes: 0

Views: 53

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 598011

There are 2 problems with your code:

  1. when creating each std::thread, you are passing a copy of each object as the this parameter of sample().

  2. Pushing multiple objects into the chains vector the way you are doing may cause the vector to re-allocate its internal array, thus invaliding any object pointers you have already passed to existing threads, since those original objects are now gone after the re-allocation.

You need to fully initialize the chains vector before creating any of the threads. And you need to pass a pointer to each object to each thread.

You can reserve() the array up front to avoid re-allocation while pushing into it, eg:

int main()  
{
    int number_of_threads{3};
    int number_of_samples{1000};

    std::vector<markov_chain> chains;
    std::vector<std::thread> workers;

    chains.reserve(number_of_threads);

    for (int i = 0; i < number_of_threads; ++i) {
        chains.push_back(markov_chain(number_of_samples));
        workers.push_back(std::thread(&markov_chain::sample, &chains[i], 99));
    }

    for(auto &t : workers) {
        t.join();
    }

    for (auto &c : chains) {
        std::cout << c.length << ' ' << c.acceptance << std::endl;
    }

    return 0;
}

Demo

However, since all of the objects are being initialized with the same starting value, an easier way is to simply get rid of chains.push_back() altogether and use chains.resize() instead, eg:

int main()  
{
    int number_of_threads{3};
    int number_of_samples{1000};

    std::vector<markov_chain> chains;
    std::vector<std::thread> workers;

    chains.resize(number_of_threads, markov_chain(number_of_samples));

    for (int i = 0; i < number_of_threads; ++i) {
        workers.push_back(std::thread(&markov_chain::sample, &chains[i], 99));
    }

    for(auto &t : workers) {
        t.join();
    }

    for (auto &c : chains) {
        std::cout << c.length << ' ' << c.acceptance << std::endl;
    }

    return 0;
}

Demo

Or, even use the vector constructor itself:

int main()  
{
    int number_of_threads{3};
    int number_of_samples{1000};

    std::vector<markov_chain> chains(number_of_threads, markov_chain(number_of_samples));
    std::vector<std::thread> workers;

    for (int i = 0; i < number_of_threads; ++i) {
        workers.push_back(std::thread(&markov_chain::sample, &chains[i], 99));
    }

    for(auto &t : workers) {
        t.join();
    }

    for (auto &c : chains) {
        std::cout << c.length << ' ' << c.acceptance << std::endl;
    }

    return 0;
}

Demo

Upvotes: 1

Related Questions