is it safe to delete std::function from withing its stored callback

Question

I have a class which stores a callback in std::function and does some operations then notify the callback target with the results . if the target sees the results invaild or doesn't want to continue doing the work it will delete the caller class which contains the std::function callback .

I know if std::function doesn't access its members after deletetion this will be safe but I didin't see such guarantee . The thing I have is that the caller class doesn't access its members after invoking the callback , but the callback can choose to do more work again and in this case it won't delete the caller .

this example illustrates the problem :

using callback = std::function<void(Caller& c, bool failed)>;

class Caller
{
    callback cb;
    bool Work()
    {
        return true;
    }

public:

    Caller(const callback& cb) : cb{ cb } {}

    void DoWork()
    {
        bool result = Work();
        cb(*this, result);
    }

};


int main(int argc, char **argv)
{

    for (int i = 0; i < 1000; ++i)
    {
        Caller *c = new Caller{
            [](Caller& c, bool failed)
        {
            if (failed)
                delete &c;
            else
                c.DoWork();
        }
        };
        c->DoWork();
    }
}

I compiled the code on windows using msvc and it's working without issuses (or may be some hidden heap corruption ?)

I don't want suggestion to use another design as the real code I'm using is complicating and the caller class is like a proky between the worker and the callback and is allocated per operation , also refactoring the code will be much work

EDIT : I know it is'nt the best c++ practice but I'm dealing with c api particularly iocp. my code works like this : the io object (socket) issue an io request which requires allocating memory for the overlapped structure and op data , this structure will hold the callback and when the op is done it will invoke the callback and if there is an error the callback may not continue otherwise it will issue the next request . after invoking the callback the structure will delete itself and the new operation started from the callback will continue like this .

now suppose I have a sessionn of half-duplex protocol, like http, then there is only one operation at a time so I may use the same stucture for all operations without allocating again , so when the second operation starts I destruct the old strucutre and construct in place the new one and this is done inside the callback of the first structre

Why don't you use shared_ptr here? This way you wouldn't have to delete anything. — Igor R., Dec 21 '19 at 11:48
Have you read this? https://stackoverflow.com/questions/22998364/deleting-a-stdfunction-object-within-itself — John Zwinck, Dec 21 '19 at 11:49
Does this answer your question? [Is delete this allowed?](https://stackoverflow.com/questions/3150942/is-delete-this-allowed) I think this is equivalent as `delete &c;` should be the same as `delete this;` — Richard Critten, Dec 21 '19 at 12:21
Even if it is not a UB and it is operational code - you shouldn't do it. You contradict C++ guidelines on several occasions. You'd better do extra work and refactor now instead of continuing to write code in such manner and creating an even bigger unmanageable abomination. — ALX23z, Dec 21 '19 at 14:07

score 0 · Answer 1 · answered Dec 21 '19 at 14:53

0

Looks OK. I would replace Caller& reference in the callback with a pointer, but that’s not important.

The only potential issue I see in your code, the recursion you have there can cause stack overflow. If your Work() function will return false = not failed few hundred thousand times (or just 600 times in debug builds, they consume much more stack space due to the lack of optimizations), your app will exhaust the stack and crash. If you know your Work() function only returns false a few times before returning true, you’re good. But if your work function needs to be able to run many iterations, consider replacing recursion with a loop.

answered Dec 21 '19 at 14:53

Soonts

20,079
9
57
130

yes I'm aware of this stack overflow because I had it in the debugger while writing the example . but yeah this is only an example which is very far from the real code – dev65 Dec 21 '19 at 14:56
@dev65 BTW, the reason why I recommended replacing reference with pointer – it’s easy to forget the `&` when writing your lambdas, then it will crash trying to delete a stack-allocated value. C++ doesn’t auto-casts values to pointers so if you’ll forget the `*` it won’t compile. – Soonts Dec 21 '19 at 15:00

is it safe to delete std::function from withing its stored callback

1 Answers1