The first rule of sane lambdas is to use [&] when your lambda lifetime (and all of its copies) will NOT leave the current scope.
Ie:
ForEachChicken( [&](auto& chicken){ chicken.LayEgg(); } );
you are using a lambda as a "block of code you can call", and injecting it into some code that wraps it.
The second rule of sane lambdas is to never use [&] if your lambda (or any copy of it) could possibly out-live the current scope.
std::function<void()> wrapper = [&](){f();}; // <---- What happens when object is destroyed before call?
this is an example of a lambda that could out-live the current scope. The use of [&] should get an instant -2 code review, regardless of if the resulting code is correct or not, because [&] puts way way way too much work on doing everything exactly correct.
So the first issue is to explicitly capture everything you need into the lambda.
std::function<void()> wrapper = [this](){f();};
the point of this is that it makes your problem clear -- this wrapper stores a copy of a this pointer. Its correct use then requires that it be the right pointer, be in lifetime, etc.
A bug you now have is that wrapper will be copied on operator= and on copy/move construction, but the this captured will be from the other instance. By storing a pointer-to-self within an object, you basically have to block copy-move assignment-construction, or rewrite them.
The second problem is the lifetime problem you noted.
Fixing the first problem, I'd make wrapper be a method itself:
std::function<void()> wrapper() const{ return [this](){f();}; }
so now it no longer takes up state, and messes with our copy/move semantics.
For the second problem (which you came here to ask about), well this is C++. You are responsible to handle object lifetimes.
I have found 2 common patterns to deal with this. The first is if the object in question is either single-threaded, or will only be invoked and destroyed from the same single thread. The second is if the object in question has a complex lifetime I want to be managed by shared pointers.
If the object in question is single-threaded (with regards to this invocation and its eventual destruction), I add a lifetime tracker to it. A lifetime tracker looks like this:
struct IsAlive {
std::weak_ptr<void> tracking;
explicit operator bool() const { return !tracking.expired(); }
};
struct LifetimeTracked {
std::shared_ptr<void> track = std::make_shared<bool>(true);
LifetimeTracked() = default;
LifetimeTracked(LifetimeTracked const&):LifetimeTracked() {} // not default
LifetimeTracked(LifetimeTracked&&):LifetimeTracked() {} // not default
LifetimeTracked& operator=(LifetimeTracked const&)&{
return *this; // not default
}
LifetimeTracked& operator=(LifetimeTracked &&) {
return *this; // not default
}
IsAlive TrackLifetime() const { return {track}; }
};
you stick a LifetimeTracked into a struct or inherit from it. Then you call TrackLifetime and store the IsAlive. You can then query IsAlive to see if the object has been destroyed.
Your wrapper() then becomes:
std::function<void()> wrapper() const{ return [this, alive=TrackLifetime()](){if(alive)f();}; }
So long as deleting the object never happens in a different thread than the return of wrapper() is called in, this will work. It doesn't require the object live in the heap, it doesn't mess with the objects lifetime so you can use it as a RAII class, etc.
I've even used this outside of lambdas in menu handling code. When the menu handler could clean up the stack of submenus or not, after the handling code you don't know if this is still around (the menu that invoked the menu item). So you grab a lifetime tracker before invoking the handler, and afterwards if it says you are dead you just exit without doing cleanup (as this is now a dangling pointer).
Now, the other option is if your class is already being managed via something like a shared pointer, or if you don't mind the heap-allocation requirements and complex lifetime of making it so.
Then you store a weak_ptr or framework alternative in your callback:
std::function<void()> wrapper() const{ return
[wp = std::weak_ptr(shared_from_this())](){
if(auto self = wp.lock())
self->f();
};
}
The downsides are that the object needs to be managed by a shared pointer, and doing so makes code compile and dangling pointers less likely, but leaks infinitely more often and in ways you won't easily be able to understand. It also makes object lifetime very difficult to understand, blocks use in automatic storage (like in a std::vector of packed objects, very efficient).
The third option is ... manage your objects lifetime. Actually guarantee that the object lives longer than any callbacks that contain pointers into it by having a full understanding of the lifetime of every object in every situation. This is insanely hard in a modern application, but I leave it here for completeness.
A forth option is to not store pointers, but only store ways to find an object. In this case, objects will have identities and probably breadcrumbs describing what they are and how to find them.
std::function<void()> wrapper() const{ return
[identity = GetIdentity()](){
if(auto self = identity.Resolve(CurrentScope()))
self->f();
};
}
Now instead of storing a pointer to the object, you somehow store a description of what this object is. When invoked, you use that description to find an object that is appropriate, and then interact with it if it exists.
Imagine if your objects are mobs in a simple game. Each mob might have a guid attached to it. Your identity could then be a string saying "the mob named bob" (where bob is a guid). At invocation time you look up in the global mob map "is there a mob named bob?", and if so you provide it to be interacted with.
std::weak_ptrmight help in that regard).