Releasing memory of std::function inside the function body

120 Views Asked by At

I need to pass a std::function as a void* pointer to be executed asynchronously, so I create a std::function on the heap. Is it safe to delete the std::function object from inside the function body?

Please see the following example:

using T = std::function<void()>;

void fun(int data)
{
    T* t = new T([&t, data]()
    {
        //do something
        delete t;
    }

    ExecuteAsync(t);
}
1

There are 1 best solutions below

4
Jan Schultke On

Mistake in capturing &t

Firstly, note that capturing [&t] is a mistake. &t would dangle when fun returns. Either you'd need to allocate the std::function first, or you'd need to use C++23 explicit object member functions to make this work:

// C++17
auto* t = new std::function<void()>;
*t = [t, data] { /* do something */; delete t; };

// C++23
auto* t = new std::function<void()>([data](this auto&& self) {
    // do something
    delete &self;
};

Assuming you did one of these two things ...

Delete self in std::function

Pedantically speaking, it is not safe to do. [func.wrap.func] operator() is only said to return INVOKE<R>(...), but it's not guaranteed to consist of just a return statement that does that. Hypothetically, it would be allowed to implement it like(1):

R operator()(ArgTypes... args) const {
    R result = INVOKE<R>(f, std​::​forward<ArgTypes>(args)...);
    // read members of this object
    return result;
}

Reading the memory of an object whose lifetime has ended is obviously UB, so this would break your code. However, no standard library does this, and there is no obvious reason why anyone would.

In practice, what you're doing will work, for the same reason that delete this is allowed. See Is "delete this" allowed in C++?.


(1) A Returns paragraph only states the returned value. It does not impose further requirements on the implementation.

Further notes on design

However, if you have to ask yourself, maybe it's not a good idea from a code style perspective. If you instead did:

ExecuteAsync(std::move(my_function));

... where ExecuteAsync obtains ownership of a std::function and also destroys it once executed, it would be much more obvious how and why it's correct. At the very least, you could hand ExecuteAsync a std::unique_ptr.