Multi-threading does not work correctly using std::thread (C++ 11)

Question

I coded a small c++ program to try to understand how multi-threading works using std::thread. Here's the step of my program execution :

1. Initialization of a 5x5 matrix of integers with a unique value '42' contained in the class 'Toto' (initialized in the main).
2. I print the initialized 5x5 matrix.
3. Declaration of std::vector of 5 threads.
4. I attach all threads respectively with their task (threadTask method). Each thread will manipulate a std::vector<int> instance.
5. I join all threads.
6. I print the new state of my 5x5 matrix.

Here's the output :

42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42

42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42

It should be :

42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42
42 42 42 42 42

0 0 0 0 0
1 1 1 1 1
2 2 2 2 2
3 3 3 3 3
4 4 4 4 4

Here's the code sample :

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

class       Toto
{
public:
    /*
    ** Initialize a 5x5 matrix with the 42 value.
    */
    void    initData(void)
    {
        for (int y = 0; y < 5; y++) {
            std::vector<int> vec;

            for (int x = 0; x < 5; x++) {
                vec.push_back(42);
            }
            this->m_data.push_back(vec);
        }
    }

    /*
    ** Display the whole matrix.
    */
    void    printData(void) const
    {
        for (int y = 0; y < 5; y++) {
            for (int x = 0; x < 5; x++) {
                printf("%d ", this->m_data[y][x]);
            }
            printf("\n");
        }
        printf("\n");
    }

    /*
    ** Function attached to the thread (thread task).
    ** Replace the original '42' value by the another one.
    */
    void    threadTask(std::vector<int> &list, int value)
    {
        for (int x = 0; x < 5; x++) {
            list[x] = value;
        }
    }

    /*
    ** Return a sub vector reference according to the range.
    */

    std::vector<int>    &getDataByRange(int range)
    {
        return (this->m_data[range]);
    }

    private:
        std::vector<std::vector<int> > m_data;
};

int         main(void)
{
    Toto    toto;

    toto.initData();

    toto.printData(); //Display the original 5x5 matrix (first display).

    std::vector<std::thread> threadList(5); //Initialization of vector of 5 threads.

    for (int i = 0; i < 5; i++) {  //Threads initializationss

        std::vector<int> &vec = toto.getDataByRange(i); //Get each sub-vectors reference.
        threadList.at(i) = std::thread(&Toto::threadTask, toto, vec, i); //Each thread will be attached to a specific vector.
    }

    for (int j = 0; j < 5; j++) {
        threadList.at(j).join();
    }

    toto.printData(); //Second display.
    getchar();

    return (0);
}

However, in the method threadTask, if I print the variable list[x], the output is correct. I think I can't print the correct data in the main because the printData() call is in the main thread and the display in the threadTask function is correct because the method is executed in its own thread (not the main one). It's strange, it means that all threads created in a parent processes can't modified the data in this parent processes ? I think I forget something in my code. I'm really lost. Does anyone can help me, please ? Thank a lot in advance for your help.

Answer 1

It seems to me that many of the choices you've made in writing this are making your job substantially more difficult than necessary. In particular, your toto class seems (to me) to make client code more complex rather than simpler, and fails to encapsulate the data it manipulates. I think if I were going to do this, I'd write code more along these general lines:

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

std::ostream &operator<<(std::ostream &os, std::vector<int> const &d) {
    for (auto const &v : d)
        os << v << "\t";
    return os;
}

std::ostream &operator<<(std::ostream &os, std::vector <std::vector<int>> const &d) {
    for (auto const &v : d)
        os << v << "\n";
    return os;
}

int main(void) {
    std::vector<std::vector<int>> d(5, std::vector<int>(5, 42));

    std::cout << d;

    std::vector<std::thread> threads;

    for (int i = 0; i < 5; i++)
        threads.emplace_back([i, &d]() {std::fill(d[i].begin(), d[i].end(), i); });

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

    std::cout << "\n" << d;
}

Answer 2

I resolved my problem. All comes from the line :

threadList.at(i) = std::thread(&Toto::threadTask, toto, **vec**, i);

which must be :

threadList.at(i) = std::thread(&Toto::threadTask, toto, **&vec**, i);

and my function 'threadTask' have to takes a pointer (not a reference) in first parameter.

void    threadTask(std::vector<int> *list, int value)
    {
        printf("b=%p\n", list);
        getchar();
        for (int x = 0; x < 5; x++) {
            (*list)[x] = value;
        }
    }

I hope this example will help somebody.