6

Is it undefined behavior to be destroying/deleting a std::function while in the middle of invocation? 

class Event {
  public:
    Event(std::function<void()> f) : func(std::move(f)) {}
    ~Event() {}
    std::function<void()> func;
};

int main()
{
    std::vector<Event> events;
    auto func = [&]() {
      events.pop_back();  
      std::cout << "event" << std::endl;
      // do more work  
    };

    events.emplace_back(std::move(func));
    events[0].func();

    return 0;
}
Mozbi
  • 1,399
  • 1
  • 17
  • 25
  • 1
    OT: why not make something else take care of that, possibly after invocation? Do you really want to write `pop_back` in every event handler? – LogicStuff Aug 09 '18 at 20:25
  • You're not doing anything "in the middle". Everything you do is sequenced. – Kerrek SB Aug 09 '18 at 20:25
  • 2
    I'm sure it's UB if you access objects in a lambda's capture after it's been destroyed, since the captured objects would also have been destroyed. I'm not sure about the general case. – alter_igel Aug 09 '18 at 20:27
  • 1
    Does `// do more work` use any of the lambda's members? – NathanOliver Aug 09 '18 at 20:28
  • 4
    Possible duplicate: https://stackoverflow.com/questions/22998364/deleting-a-stdfunction-object-within-itself – Rakete1111 Aug 09 '18 at 20:34

2 Answers2

6

This is undefined by [res.on.objects]p2:

If an object of a standard library type is accessed, and the beginning of the object's lifetime does not happen before the access, or the access does not happen before the end of the object's lifetime, the behavior is undefined unless otherwise specified.

The "access" in this case consists of the call to the function call operator of the std::function. The std::function object's lifetime ended at the pop_back() call, in the middle of the access. Therefore, the access does not happen before the end of the object's lifetime, and the behavior is undefined.

T.C.
  • 133,968
  • 17
  • 288
  • 421
  • Discussed once the case where an object deleted itself in one of its functions. Common sense then was that it is OK as long as the function does not access any members any more *afterwards*. Background: function code and the object itself are at separate locations, so even if within the member function, the pure call does not access the object. Critical here is the reference to the vector, though. But it is copied (the reference, not the vector...) into pop_back argument, so should not be a problem (would, though, if the vector was captured by *value*!!!). – Aconcagua Aug 10 '18 at 06:38
  • If the discussion of those days *was* correct, then the removal from vector should be fine *if and only if* it is the last action that references any object members (i. e. the members of the closure). Otherwise, it would mean that self-deletion from within a function in general is not possible... – Aconcagua Aug 10 '18 at 06:38
  • It doesn't matter what core says. You are using a library component, and you need to follow the library's rules. – T.C. Aug 10 '18 at 06:54
  • And where don't I do so with above considerations? The vector is captured by reference, the reference is passed to pop_back. Fine, pop_back's `this` points to the vector in main. At some point, the lambda's destructor is called. Fine. The destructor does not delete the vector, as only a reference. Fine. The lambda does not access any captured members any more either. Fine. So then, where's the problem (unless self-deletion in general is UB!)? – Aconcagua Aug 10 '18 at 07:00
  • Hm, OK; there's the `// do more work comment`- if there happens some member access... UB! – Aconcagua Aug 10 '18 at 07:02
  • You are not destroying just the lambda. You are destroying a `std::function`, which is a library component and so follows the library's rules. (IOW, the standard library implementers aren't required to bend over backwards to tolerate such shenanigans.) It's UB even if the lambda returns immediately after the `pop_back`. – T.C. Aug 10 '18 at 07:28
  • Hm, the `std::function` per se shouldn't be critical, it's just an ordinary member of Event class, holding the (a copy of the original) lambda, and thus is destructed right together with Event. But I think I'm starting to catch up: the critical point is that we are right somewhere within `std::function`'s `operator()`, and we probably have no guarantees that it does not access any of its own members after calling the lambda, so we at least potentially end up in undefined behaviour, don't' we? – Aconcagua Aug 10 '18 at 07:54
  • From [cppreference](https://en.cppreference.com/w/cpp/utility/functional/function/operator%28%29): *"Effectively does `INVOKE(f, std::forward(args)...)`"* - would that suffice to introduce such forementioned guarantee? – Aconcagua Aug 10 '18 at 08:36
  • It doesn't matter. There's blanket library wording saying that accessing a standard library object after its destructor starts is UB. That supersedes whatever the core language rule is as far as the standard library is concerned. – T.C. Aug 10 '18 at 18:46
  • Found under [defns.access]: *"access 〈execution-time action〉 to read or modify the value of an object"*. My problem now is: With this definition I just cannot see where the std::function still is *accessed*, unless possibly that `operator()` accesses own members after calling the lambda. If it doesn't, then no UB. For safety we'd have to consider *potential* UB just like UB actually occurring, so far I am totally with you. So finally I get back the third time at the already mentioned guarantee: Do we have it or not? – Aconcagua Aug 15 '18 at 06:20
  • Don't want to appear stubborn, just: we have two contradicting answers, one saying OK, one UB. I currently cannot see myself where UB actually (or only potentially) occurs, unless we cannot rely on calling the lambda is the last action within `std::function::operator()`, so that's why I am that tenacious (hey, I got the batch for...) on the topic, for being able to finally make up my own mind... – Aconcagua Aug 15 '18 at 06:29
3

Your code (the bit you're asking about, i.e. the destructing of the object inside a member function) is roughly equivalent to

struct A
{
  void f() { delete this; }
}

int main()
{
  A* a = new A;
  a.f();
}

This does actually work. Refcounted resources could do something similar when the refcount reaches zero in their unref function.

Note you might want to rethink tying the event list and events themselves together like this. An event shouldn't know about its environment (the event queue).

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
rubenvb
  • 74,642
  • 33
  • 187
  • 332
  • [As long as you're careful, it's okay for an object to commit suicide.](https://isocpp.org/wiki/faq/freestore-mgmt#delete-this) – Rain Aug 09 '18 at 20:31
  • 5
    This works when you don't access `A`'s members after the `delete`. Is there a guarantee that `std::function` won't access its own members after invoking the callable? – interjay Aug 09 '18 at 20:36
  • 1
    @interjay It is legal (as in it wouldn't be UB, not sure if the standard sctually allows it to do so or not) for the `std::function` to do anything it wants with the functor. In its destructor the functor has not been destroyed yet. What would be UB would b `f()` using some state from `A` after it did `delete this;` – NathanOliver Aug 09 '18 at 20:40
  • Counter-example: imagine the vector was captured *by value*. Then we have such a case, potentially, at least, as the vector would get deleted right in the middle of pop_back call. Only if calling the destructor was the last thing pop_back does (which we have no guarantees for and is unlikely as well), we might get away without UB... – Aconcagua Aug 10 '18 at 06:56