Using smart pointers within multi-threaded application

Question

I have some calculation I need to perform that is dependent on two or more steps as follows:

class A
{
public:
    double step1() { return 2.5; }
};

class B
{
public:
    double step2() { return 1.2; }
};

class Result
{
public:
    Result(std::shared_ptr<A> _a, std::shared_ptr<B> _b) : a(_a), b(_b) {};

    double getResult() { return a->step1() + b->step2(); }

private:
    std::shared_ptr<A> a;
    std::shared_ptr<B> b;
};

In actuality, step 1 and step 2 need polymorphic behavior, so these (shared) pointers would be to the "interface" class, but that detail isn't important here.

Now, the final calculation in getResult() also needs polymorphic behavior, so I create a (unique) pointer to Result, create a lambda calling getResult(), and pass that lambda to my threads like so:

void run_multi_threaded_calculation()
{
    auto result = create_result_unique_ptr();

    const int nThreads = 4;
    std::vector<double> save(nThreads);

    auto task = [&](int n) {
        // Preprocessing before getResult()
        save[n] = n * result->getResult();
    };

    std::vector<std::thread> threads;
    threads.reserve(nThreads);
    for (size_t i = 0; i < nThreads; ++i)
    {
        threads.push_back(std::thread(task, i));
    }

    for (auto& th : threads)
        th.join();

    for (const auto& s : save)
        std::cout << s << '\n';
}

Question 1: Am I using the correct configuration of smart pointers and lambda capture, e.g. unique_ptr to Result and shared_ptr to A and B? After some guess and check changing smart pointer types the above compiles (but does not compile if a and b in Result are unique_ptr's), but I'm not sure if this is the best way to approach this.

Question 2: If I replace the lambda with an equivalent (or so I thought) function object, then my code does not compile (Error C2661: 'std::tuple<ResultFunctor,unsigned int>::tuple': no overloaded function takes 2 arguments). Is there something I'm missing with smart pointers, or maybe the way threads work, or possibly some issue with my function object definition?

Here's the relevant changes:

class ResultFunctor
{
public:
    ResultFunctor(std::unique_ptr<Result> _result, std::vector<double>& _save) : result(std::move(_result)), save(_save) {};

    void operator() (int n) { save[n] = n * result->getResult(); }

private:
    std::unique_ptr<Result> result;
    std::vector<double>& save;
};

and replace the following line:

void run_multi_threaded_calculation()
{
    // Other stuff is unchaged...

    /*auto task = [&](int n) {
        // Preprocessing before getResult()
        save[n] = n * result->getResult();
    };*/

    auto task = ResultFunctor(std::move(result), save);

    // other stuff is unchanged...
}

This is just the simplest example that demonstrates my question. My actual program will have `Result` behave polymorphically, deriving from something like`IResult` base class. `getResult` will then use step 1 & 2, but with some other different calculations. — Charlie H., Jul 20 '20 at 19:23
This might be a good use case for coroutines, especially if the order of the steps is important. In your example, I don't think you can be assured that step1 will evaluate before step2. — ttemple, Jul 21 '20 at 10:55
I'll look into coroutines, but your comment made me realize my example was slightly misleading. In reality the steps won't be evaluated in the same line but more like: `step1() /*... Do some stuff with the result...*/ step2()`. — Charlie H., Jul 22 '20 at 02:07

score 1 · Accepted Answer · answered Jul 20 '20 at 19:49

Part of the problem you are having is that you are passing unique_ptr's around. But you have missed one move for the one that resides within your ResultFunctor class when trying to pass it into std::thread here:

threads.push_back(std::thread(task, i));

If you have to use unique_ptr then you will most likely need a move c'tor in ResultFunctor:

class ResultFunctor
{
public:
    ResultFunctor(std::unique_ptr<Result> _result, std::vector<double>& _save) : result(std::move(_result)), save(_save) {};

    void operator() (int n) { save[n] = n * result->getResult(); }

    // Move c'tor rough un-tested example
    ResultFunctor(ResultFunctor&& rf) :
        result(std::move(rf.result)),
        save(rf.save)
    {};

private:
    std::unique_ptr<Result> result;
    std::vector<double>& save;
};

So that you can then move it into std::thread's c'tor which takes an r-value ref

threads.push_back(std::thread(std::move(task), i));

At least this is the main issue I can see, even if its not causing your current error.

Note: that your lambda is quite different to your "equivalent" functor class. Your lambda is capturing by reference. However that's not really so safe as when you pass that to the thread the REAL unique_ptr could go out of scopre and destroy it!

So this is not really a nice solution. In c++14 I believe you can capture the unique_ptr using move: How to capture a unique_ptr into a lambda expression? and https://isocpp.org/wiki/faq/cpp14-language#lambda-captures

This helped answer my question. I made the changes you suggested and everything compiled fine, however I got an exception (read access violation due to a `nullptr`), I think because the `unique_ptr` was deleted by one of the threads before the other ones could finish, so I think it should be a `shared_ptr`. However, I ended up using the functor with a raw pointer to result, since ownership of `Result` was managed by the function calling the functor, not the functor itself. — Charlie H., Jul 21 '20 at 02:51
@CharlieH. Good spot, yes I think having converted from a shared to a unique you lost the "sharedness". So when the reference count of the shared pointer gets to zero it would be destroyed :o — code_fodder, Jul 21 '20 at 06:14

Using smart pointers within multi-threaded application

1 Answers1