Removing std::function lambda-wrapped method from std::map

Question

I'm creating a callback system using std::function and std::maps. The map uses ints as keys and the values are std::function. I bind methods into those functions. I'm wondering if I call map.erase(i), will that delete the std::function from memory, or will I have a memory leak?

Here is some example code:

#include <iostream>
#include <functional>
#include <map>

using namespace std;

class TestClass{
    public: 
        TestClass(int _i, map<int, function<void()>>& test_map):i(_i){
            test_map[i]=[&](){this->lambda_test();};
        };
        void lambda_test(){cout << "output" << " " << i<< endl;};
    private:
        int i;
};

int main() {
    map<int, function<void()>> test_map;
    TestClass* test = new TestClass(1, test_map);
    test_map[1]();
    delete test;
    test_map.erase(1); // <-- here
  };

Does the last test_map.erase(1); remove the std::function from memory?

The only thing to watch out for in this code is between `delete test;` and `test_map.erase(1);` where the lambda is referencing a dangling `this`-pointer. In this example, this isn't harmful, but 'growing' applications or multi-threaded application could start showing undefined behavior. In general there is a weird lifetime issue in the code which could be solved by storing TestClass in the map instead of the lambda. — stefaanv, Aug 22 '16 at 14:13

score 3 · Accepted Answer · answered Aug 22 '16 at 14:09

3

While this is not good code, there is no memory leak; you're storing the std::functions in the std::map by value (rather than by pointer), hence std::map::erase will call the destructor of std::function.

Put another way, you're not newing any std::function, so you don't need to delete any std::function.

answered Aug 22 '16 at 14:09

Daniel

8,179
6
31
56

What makes it not good code? Should I store std::function pointers rather than the actual objects in the map? – Max Tyler Aug 22 '16 at 14:11
Out of curiosity, what in particular about this code is 'not good'? – sji Aug 22 '16 at 14:11
@MaxTyler Why `new TestClass`? What is `TestClass` doing at all here? I understand this is a toy example, but if you're using anything like this pattern in your actual code then I highly suspect a design flaw. – Daniel Aug 22 '16 at 14:19
3

@sji I could make up some criticisms, like lifetime isn't managed very safely. It captures `&` and grabs `this`, which luckily ignores the request to capture by `&`: use of `[&]` in a lambda that lasts beyond the current context is code smell and extremely fragile. It wouldn't be hard to go on? – Yakk - Adam Nevraumont Aug 22 '16 at 14:21
@MaxTyler Oh, and you should certainly not store the `std::function`s in the `std::map` by pointer! – Daniel Aug 22 '16 at 14:25
@Daniel I'm making a simple simulation to test out simple neural networks. It consists of entities with a unique id stored by reference in a `map`. The entities can register their functions with the event manager to act as callbacks, the form of the functions being `void func(Event* e)`. I'm storing the callbacks in a nested map `map > >, and I'm wanting a way to remove those callbacks when an entity is removed. – Max Tyler Aug 22 '16 at 14:27
@MaxTyler Why don't you ask a question on [code review](http://codereview.stackexchange.com)? You'll get much better feedback than I can give here in a comment. – Daniel Aug 22 '16 at 14:36
@Daniel I'll do that. Thanks for your help! – Max Tyler Aug 22 '16 at 14:44
2

@MaxTyler Please be aware that CodeReview questions are different from SO questions; make sure to read the guidelines! – user2296177 Aug 22 '16 at 14:45

score 1 · Answer 2 · edited May 23 '17 at 12:23

There is a very good explanation of what goes on with lambdas in terms of the actual memory allocation here:

https://stackoverflow.com/a/12203426/1230538

As I understand it, the lambda syntax creates an r-value, copied (along with any captured state etc) into the std::function. This will be deleted by the destructor of the std::function, called by the destructor of the std::map when you call erase (and/or when the map goes out of scope).

Removing std::function lambda-wrapped method from std::map

2 Answers2