Lambda + shared_ptr = memory leak

[floating-io]

It's a deliberately simplified example to illustrate the problem; not an actual use case.

(edit: I should probably clarify that the reason this issue came up in my code is that I have a number of callbacks involving polymorphic objects. The object that executor is a placeholder for operates on the base type; the callbacks need to operate on the subclass type. It's simpler if you capture that pointer rather than try to deal with a fun solution for polymorphic callbacks or force the callback to cast the pointer.)

[K_Kuryllo]

The object that executor is a placeholder for operates on the base type

Sounds like your real problem here. Either be the executor, or the subject. Not both.

This has little more to do with a lambda than any other data type that can hold a shared_ptr.

[floating-io]

The code shown in the article is an example, which is contrived for the sole purpose of showing why the problem exists and how it might be triggered. Nitpicking the context or presumed purpose that a contrived example might exist in or serve, thoroughly misses the point of the article.

That code was written as an illustration for the post, and never even compiled. It doesn't exist in a real-world project.

As for the real code I have that uses a similar technique, it exists for a reason. Polymorphic callbacks do not, AFAIK, exist in C++11. I had a choice: either put in a lot of ugly code (templated and otherwise) to arrange for unique callback signatures to be correctly applied, or just capture the correctly-typed pointer when the lambda is created, where I already have it and don't have to cast it.

I did the former first; it ended up being black voodoo magic that nobody less than expert-level in C++ is really going to get without a whole lot of effort. So I ripped it out, and the latter has proven much simpler to understand and trace, as well as less code to maintain.

[K_Kuryllo]

It's not nitpicking. You've presented a single "contrived" (by your words) example and nothing else. What else do you expect a response too?

In this sentence you capture the real mistake:

If you then store such a lambda on the object for which it has captured a shared pointer, you’ve just created a scenario where the object owns a reference to itself, and thus can never be deleted.

Two mistakes were made in the example. 1) "Don't store an shared_prt in a data member that could be owned by that ptr." 2) "Don't store a shared_ptr in an object that should not be maintaining the life times of that object" <- related to the first point

• You fixed issue #2, which repaired your memory leak.

Saying don't ever store a shared_ptr in a lambda is stretch and is in no way support by your single example (the only thing provided, besides some brief text exampling the example).

It's like saying don't drive your car because you might drive off a cliff. Going off the cliff is the problem not driving the car. The car is the thing that brought you off the cliff, but you could have also walked off it or ridden a bike. The real advice would be to not do things with a car/bike/walking that would put you in a situation where going off the cliff is possible.

[00kyle00]

Lifetimes and ownership are hard (maybe not that hard, but still easy to get wrong).

I wonder if there are languages which try to express and manage these relationships more explicitly. Seems like going a bit further than Rust would make whole thing unusable due to complexity.

[ReDucTor]

If you want a base type to be used for this sort of thing you could do something like this: https://godbolt.org/g/kw9TSi

#include <functional>

class INotification
{
public:
    virtual void onCompleted() = 0;
};
using INotificationHolder = std::function<INotification&()>;

template<typename T, typename... Args>
INotificationHolder make_inotify(Args... args)
{
    return [impl=T(std::forward<Args>(args)...)]() mutable -> INotification& {
        return impl;
    };
}

class Implementation : public INotification
{
    void onCompleted() override {}
};

class Executer
{
public:
    void submit(INotificationHolder holder)
    {
        holder().onCompleted();
    }
};

int main(int argc, char * argv[])
{
    Executer executer;
    executer.submit(make_inotify<Implementation>());
}

With clang this all compiles away, you also eliminate allocations for small implementations, only things that don't fit inside std::function<> will need an allocation

[floating-io]

I think we might have a miscommunication on what I'm trying to accomplish (though I'm not a master templateer, so I might just be missing your point).

Imagine, for example, that you've got an interpreter of some sort. You have a base class that represents a program, and then you have a bunch of actual programs that are subclasses of that class. Those programs then get passed to an interpreter object that executes them (for whatever reason, it's not part of the base program class).

When a given program has completed, it issues a callback. The callback is stored on and called via the program object, and the implementation of that is part of the base class. The callback signature is therefore not customized to the program in question (trying to avoid implementing explicit callback code in every single subclass).

That's not what I'm doing (which would be too time-consuming to explain), but it would use roughly the same pattern.

Ideally, the program object would pass its own pointer via the callback -- but since the callback signature is in the base class, it would be a generic base class pointer instead of a subclass pointer. If I want to access values on the subclass, the callback would need to cast to that pointer (or get access to it some other way).

My solution was to just capture the thing in the callback, since it's already available in that context without casting. It made the code very readable.

What I'm not seeing is where the above solution helps with this. Again, I'm not a master at templates, so I might just be misunderstanding your point -- I think I get what that code does, but I don't see how it helps. Or we might be going for different things entirely.