9

I know already that some forms of "suicide" are safe (to be considered legal), but, is it specifically safe to perform delete this in a virtual member function?

Note, by "safe", I mean whether the "code" generated by the compiler is able to deal with the construct.

Note, I'm not interested in the pros and cons of doing it, just whether I can consider is safe.

Side question: Does the language standard explicitly or implicitly demand that implementations support any forms of the delete this idiom?

I do not consider this a duplicate of Is delete this allowed?. My question is about whether it is safe to do in a virtual member function.

Here is an outline of what I am pursuing to do

class FooBase {
protected:
    virtual void on_idle() { /* no-op by default */ }
};

class Foo : public FooBase {
    void on_idle() override final
    {
        delete this;
    }
};

Note that while Foo needs to be heap allocated, other subclasses possibly do not.

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
Kristian Spangsege
  • 2,903
  • 1
  • 20
  • 43
  • 2
    how can we tell you whether it's safe when we don't know in which context you're doing it? In general, `delete this` is a bad idea (indeed, exceptions where it can be OK might exist), but if in doubt, sounds like a bad (and unsafe) design decision. Maybe if you explain why you want to do that, a more definite answer can be given. (But you'll also, inevitably, hear about the cons of doing it, and honestly, I think deservedly so, because software design usually happens within the semantics of a language, and C++ was never meant to `delete this`). – Marcus Müller Jan 03 '18 at 21:45
  • 1
    I would not think so, I would be concerned about where the `vtable` goes afterwards. But I'm interested int he answers, so I'll leave this comment – excalibur1491 Jan 03 '18 at 21:46
  • 2
    Considering that COM objects implement interfaces with virtual methods, and `delete this` is almost always written in an overriden `IUnknown::Release()` method, then I would have to say yes, it is safe. Tons of Microsoft/Windows technologies depend on it. – Remy Lebeau Jan 03 '18 at 21:46
  • 1
    There are a lot of gotchas, but the vtable is not one of them. You should search before you post a question. Here's one https://stackoverflow.com/questions/3150942/is-delete-this-allowed – Kenny Ostrom Jan 03 '18 at 21:50
  • @RemyLebeau That doesn’t mean it’s safe on a non-Windows system. – Daniel H Jan 03 '18 at 21:53
  • 3
    Generally whether a function is virtual or not only matters when it's called, not when it returns. – Mark Ransom Jan 03 '18 at 21:55
  • 1
    @RemyLebeau COM also depends extremely heavily on being able to arbitrarily switch between `void**` and `IWhatever**`, relying on the same representations for all pointer types and for the compiler to not optimise in ways that break such aliasing. (Yes, that's how MS documents it, that's not just programmer negligence. See e.g. https://msdn.microsoft.com/en-us/library/windows/desktop/ff485837(v=vs.85).aspx) Don't think "COM relies on this" means "this is okay in standard C++". COM relies on implementation extensions. –  Jan 03 '18 at 21:56
  • 1
    I'd be tempted to say that asking specifically about virtual functions makes this question different than the linked duplicate. However I think we can assume that if the answers to that question don't mention virtual functions then it doesn't matter. – Mark Ransom Jan 03 '18 at 22:00
  • 1
    @MarkRansom I was conflicted. Let's let it live? – Drew Dormann Jan 03 '18 at 22:14
  • @MarcusMüller I added some info about what I am trying to do. – Kristian Spangsege Jan 03 '18 at 22:26
  • @KristianSpangsege thanks! Now, a really good question! Upvote! – Marcus Müller Jan 03 '18 at 23:15

1 Answers1

15

Yes, so long as you don't use this afterwards, and neither does anyone else with a pointer to *this, and this was guaranteed to be allocated by new as exactly the type you are deleting it as, or possessing a virtual destructor. (ie, never as a member of another object, in a std::vector, as an automatic storage variable, as a static variable, as a temporary, not new[], not placement new, etc etc etc)

This includes calling non-virtual methods, virtual methods, member access, calling dtors, and a myriad of other things; almost anything other than return; on the next line and somehow every other pointer to *this being cleaned up before you delete this; (or deterministically never being used).

As a general rule, the level of control you have to have over your objects lifetime is so great to make delete this; safe that you can refactor the lifetime management to be external to the class and in a smart resource owner, which maybe maintains its state as a pImpl which it deletes. adores value types, and a type that delete this; can never be treated as a value.

There is nothing in the standard that makes delete this; extra dangerous for virtual objects, other than the higher tendency to inherit.

All types that delete this; should have either a virtual destructor or be final to avoid inheritance issues.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • 9
    and make sure this was created by new – Kenny Ostrom Jan 03 '18 at 21:52
  • 1
    And not `new[]`, but those are standard precautions necessary in order to make `delete this;` safe even for non-virtual objects. – Daniel H Jan 03 '18 at 21:55
  • Safe? Maybe, under very controlled circumstances. Good practice? probably not. Are you likely to bump into problems? Unless the code is super trivial then yes. – Martin York Jan 03 '18 at 22:24
  • Keep in mind that in OO designs, it's pretty common for a subclass to implement its own override of the virtual method, and have its override-method "call up" to the superclass-method, and then do something additional after the superclass-method returns. Most subclass-method authors are going to be very surprised if their this-object gets deleted out from underneath them during the superclass-method-call, and therefore they are likely to do something that results in undefined behavior or a crash afterwards. So even if initially implemented correctly, "delete this" is a bug waiting to happen. – Jeremy Friesner Jan 04 '18 at 01:25
  • @Jeremy - if a inherited function is going to do `delete this`, then that fact should be carefully documented. And the derived version can only do things BEFORE calling the inherited version. It can't do things which access the object (non-static members) after that. – Peter Jan 04 '18 at 01:27
  • @Peter agreed... and even then there will be a lot of developers who don't read the documentation, at least not until after they've already been burned :) – Jeremy Friesner Jan 04 '18 at 01:28