2

I know if I have to call member functions in lambda for callbacks, I capture this in lambda and it works well. But I have seen some crash recently and it appears that the member function is accessed after the object pointed to by this is already destroyed. In nutshell, in shutdown the object is destroyed, but the pointer was passed around in lambda, which is accessed and results in crash.

So, I was trying to understand what the community generally do in such a case. I could not find much to this but I think shared_ptr could be an option.

Any suggestions/leads would be appreciated to help me understand and implement an alternative.

H.S
  • 53
  • 5
  • 1
    Can you post a code snippet? – The Eyesight Dim Dec 01 '16 at 20:17
  • You can inherit from `std::enable_shared_from_this`, use `shared_from_this()` to get a shared pointer and either capture a `std::shared_ptr`if you want to be sure that the object is still alive or a `std::weak_ptr` if you want to check if it's alive or not before to do any operation. – skypjack Dec 01 '16 at 20:19
  • I'll one-up that @TheEyesightDim. Could we get a [mcve]? – user4581301 Dec 01 '16 at 20:21
  • `[self = *this]() { self.f(); }`? – Kerrek SB Dec 01 '16 at 20:22
  • @KerrekSB You must guarantee that the class has a proper constructor in this case. It isn't always the case. Of course, it isn't always the case that you can modify it to inherit from `std::enable_shared_from_this` too. – skypjack Dec 01 '16 at 20:29

2 Answers2

6

In C++ you are responsible to track object lifetimes.

This means you have to track the lifetimes of things that hold pointers and references to other things, and ensure they do not live as long as those things.

You failed in your task. You passed lambdas that capture pointers to objects around as if they where candy, and not a direct line into the guts of your object.

Solving lifetime problems by sprinkling shared pointer around is usually a bad idea. Making the lifetime of your objects more nebulous may reduce the incidents of immediate crashes, but a nebulous ball of object lifetimes does not make your program work. The nebulous ball either expands to encompass your entire program, which now can never actually shut down, or it circles back in on itself and self-perpetuates, leaking resources.

Shared pointers can be used in narrow situations where you have a defined lifetime relationship that is best modelled as a shared ownership. This is not at all the same as "I have objects going away before their pointers, so I should try shared pointer!" You have an object lifetime problem. You try shared pointer. Now you have two problems: the original object lifetime problem, and your shared pointer problem.

Callbacks are an example of a case where you need strict lifetime rules. How long do you callback? When do you stop? How urgently you recycle resources? How do you unregister the callback? Etc.

I have written callback systems that use shared and weak pointers. They aren't perfect. Here is one I found in google: broadcaster. The listener stores tokens to say "keep talking to me", when they go away the broadcaster stops yammering at it.

Community
  • 1
  • 1
Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • a locked weak_ptr is still the most reliable and easy way to eliminate the crossing case where the subscriber goes away while the unfulfilled callback lurks in the message queue. – Richard Hodges Dec 01 '16 at 20:57
  • @RichardHodges Note that `broadcaster` does that, where the 'queue' is the vector of targets being broadcast to. – Yakk - Adam Nevraumont Dec 01 '16 at 21:01
0

This is the pattern I use for handling subscriptions. The use of locked weak_ptr eliminates the risk of a crossing case.

#include <memory>
#include <chrono>
#include <thread>
#include <mutex>

using namespace std::literals;

// some external service that will send us events in a callback.
// normally of course we'd have some means to turn these off too.
// However for this demo we'll ignore that for now
void subscribe_for_events(std::function<void()> f);

struct might_go_away : std::enable_shared_from_this<might_go_away>
{
    static std::shared_ptr<might_go_away> create() {
        auto p = std::make_shared<might_go_away>();
        p->start();
    }

    might_go_away() {}

private: 

    using mutex_type = std::mutex;
    using lock_type = std::unique_lock<mutex_type>;

    // handy helper to deliver a weak pointer to ourselves
    auto weak_self() { return std::weak_ptr<might_go_away>(shared_from_this()); }

    // do startup things here, like subscribing to other services etc
    void start() {
        subscribe_for_events([this, weak = this->weak_self()]
        {
            // don't touch 'this' until we have successfully locked the weak ptr
            if (auto self = weak.lock()) {
                // we know we're alive. the variable 'self' will hold the strong count > 0
                // this is a good place to take locks
                handle_event(lock_type(mutex_));
            }
        });
    }

    void handle_event(lock_type) {
        // do things when notified by some event source.
        // we are safe here. `this` will not go away and we own the mutex 
        // we will release the lock when this function exits through RAII.
        // PLUS, because the lock was moved in, we own it. We can release it early if we wish.
    }

    mutex_type mutex_;
};

Of course in production code, the shared_ptr would be wrapped in a consumer-facing handle class.

We prefer value semantics where possible.

struct active_thing
{
    using implementation_class = might_go_away;
    using implementation_type = std::shared_ptr<implementation_class>;

    active_thing() : impl_(implementation_class::create()) {}

    // methods here

private:
    implementation_type impl_;
};


int main()
{
  {
    auto a = active_thing();
    std::this_thread::sleep_for(5s);
  }
}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142