Expand F.53: Please provide safe alternatives to capturing 'this' for lambdas destined for another thread

[borbmizzet]

Regarding F.53, my co-worker @joelspadin and I have been discussing the drawbacks of capturing 'this' from class Foo in a lambda such as

void Foo::process() { ... }
void Foo::bar() { thread_pool.queue_work([this]{ process(); }); }

As 'this' is a pointer scoped to the lifetime of the calling object, if 'this' gets destroyed before or during the lambda invocation, it falls into the issue of "Pointers and references to locals shouldn’t outlive their scope." The best solution we came up with would be something along the lines of what I have below, but it would be nice to have a core guideline that describes how to safely manage the lifetime of an object that captures itself onto another thread.

• Inherit std::enable_shared_from_this<Foo>
• Ensure Foo is always constructed with std::make_shared<Foo>() and owned as a std::shared_ptr<Foo>
• Capture shared_from_this() when queueing a critical lambda on another thread (If the lambda must absolutely be run)
• Capture weak_from_this() when queueing a non-critical lambda on another thread (If the lambda doesn't need to be run after the calling object has gone out of scope)

[shaneasd]

Is this a problem (and possible solution) for pointers in general? I don't see what makes this special. For example why should the treatment of this be different to x in this code?

#include <functional>

struct A
{
    void DoStuff() {}
    auto MakeLambda(std::vector<int>* x) { return [this, x] { x->size(); this->DoStuff(); }; }
};

int main()
{
    std::function<void()> lambda;
    {
        std::vector<int> x{4};
        A a;
        lambda = a.MakeLambda(&x);
    }
    lambda(); //UB accessing x and this

    return 0;
}

[joelspadin]

Yeah, the code we were discussing that prompted this uses this, but I believe the problem is general for pointers and references.

To reword the original question, F.53 says we should avoid capturing pointers and references for lambdas that will be used non-locally, but what if you can't avoid that? What is the safest way (if there is such a thing) to capture a pointer/reference to an object in a lambda that must be used non-locally?

Capturing a weak_ptr seems like it would work, though it requires the lambda be able to handle the object no longer existing. Assuming there aren't any reasons weak_ptr is a bad idea, shared_ptr could work too, except that you'd need to be careful to avoid circular references.

[shaneasd]

That doesn't sound like a problem specific to lambdas though right? Indeed various smart pointer solutions aim to solve this sort of problem by making ownership (and thus lifetime) more explicit.

I think the point of F.53 is that it is dangerous to capture [&] because it would be easy to retain the reference beyond the lifetime of the original object if the lambda survives longer that the original object. This is quite a specific (to lambdas) case of the more general idea of not using stuff after its been destroyed (for which smart pointers are a solution). Note also that F.52 is the complement for F.53 in that is says you should capture by [&] if you know the lifetime is appropriately limited.

[hsutter]

Editors call: While intrusively requiring shared_ptr can be a solution in some cases, we don't think it's something we should say as general advice because that means aliasing and having to synchronize to avoid race conditions and such. Consider capture using [*this] where taking copies can be appropriate -- usually passing data to another thread is best done with sending copies anyway. Or consider moving a unique_ptr into the queue to hand over ownership to the other thread, which handles both lifetime and synchronization. We'll look at putting a note like this into the Guidelines. Thanks!