17

Possible Duplicate:
Is it safe to delete this?

I've been doing a little work on a class that's designed to act as a node in a linked list, and I figured I'd give the class its own deletion function as opposed to the managing class doing it. So basically it goes like this:

void Class::Delete() {
    //Some cleanup code before deleting the object
    delete this;
}

Now I've tested this and it appears to work fine, but I've had a problem in the past where objects have been in the middle of running code, been deleted, then obviously crashed the program by trying to use a no-longer-existing object.

Since "delete this" is right at the end of the function, it obviously exits the function and works fine, but is this sort of practice a bad idea at all? Could this ever blow up in my face if I'm not careful?

Community
  • 1
  • 1
Linkage
  • 307
  • 2
  • 6
  • It's OK to do this, but as you've discovered previously it's not necessarily safe to do. You can't interact with the object anymore. Makes sense for thread objects that run and then die by themselves. If nothing holds a reference to them that might be ok. – hookenz Jun 14 '11 at 05:21

5 Answers5

22

The FAQlite answers this quite well:

As long as you're careful, it's OK for an object to commit suicide (delete this).

Here's how I define "careful":

  1. You must be absolutely 100% positive sure that this object was allocated via new (not by new[], nor by placement new, nor a local object on the stack, nor a global, nor a member of another object; but by plain ordinary new).
  2. You must be absolutely 100% positive sure that your member function will be the last member function invoked on this object.
  3. You must be absolutely 100% positive sure that the rest of your member function (after the delete this line) doesn't touch any piece of this object (including calling any other member functions or touching any data members).
  4. You must be absolutely 100% positive sure that no one even touches the this pointer itself after the delete this line. In other words, you must not examine it, compare it with another pointer, compare it with NULL, print it, cast it, do anything with it.

Naturally the usual caveats apply in cases where your this pointer is a pointer to a base class when you don't have a virtual destructor.

Basically, you need to take the same care as you do with deleteing any other pointer. However, there are more areas where things can go wrong with a member function committing suicide, compared with an explicitly-declared pointer.

seaotternerd
  • 6,298
  • 2
  • 47
  • 58
Dominic Gurto
  • 4,025
  • 2
  • 18
  • 16
  • 2
    I'm curious.. why #4? Printing a pointer etc doesn't access the underlying memory at all. – ThiefMaster Jun 14 '11 at 06:09
  • 1
    Out of curiosity, why must you not read the value of `this` after `delete this;`? It seems to me that this would be an ordinary deletion under any circumstances, after which reading is allowed (though of course nigh useless). Is it explicitly forbidden in the standard, or is the FAQ slightly overcautious? – Jon Purdy Jun 14 '11 at 06:10
  • 1
    "If the operand to the delete operator is a modifiable l-value, its value is undefined after the object is deleted." - http://msdn.microsoft.com/en-us/library/h6227113.aspx – Dominic Gurto Jun 14 '11 at 06:13
  • Also http://stackoverflow.com/questions/1866461/why-should-i-not-try-to-use-this-value-after-delete-this – Dominic Gurto Jun 14 '11 at 06:17
  • 1
    +1. One way to make the Delete() implementation safer (point #4) is for it to require a reference to the caller's pointer so that it can force it to NULL, and they cannot hold on to the dead pointer. (Obviously they could have a copy of the pointer elsewhere, but in most cases we hold a single master pointer and as long as this is NULLed then you can be sure the program won't access the object's memory in future) – Jason Williams Jun 14 '11 at 06:19
  • That would solve that problem, and prevent some potential undefined behaviour, but it would be way too restrictive for the delete operator to require a non-const reference to a pointer. Technically the non-const version of the operator could do it, but that would mean the behaviours of the two versions of the operator would be different, which isn't really a good idea. – Dominic Gurto Jun 14 '11 at 06:27
  • 2
    Just because it's in the FAQLite doesn't mean it's true. Every point mentioned applies equally well to all deletes. There's nothing special about `delete this` (and in fact, in many applications, it will represent the majority of the deletes). – James Kanze Jun 14 '11 at 08:04
  • @Jason Williams There's nothing about nulling the pointer which gets deleted which makes things safer. Presumably, the function which calls `Delete` knows the semantics of the function, and will not reuse the pointer. Any danger (and that holds for all deletes, not just `delete this`) resides in other pointers to the object, not in the pointer held by the caller. – James Kanze Jun 14 '11 at 08:06
  • 4
    @Jon According to the standard, any lvalue to rvalue conversion of an invalid pointer is undefined behavior. And deleting an object renders *all* pointers to it invalid. (This has nothing to do with `delete this`. It holds systematically.) – James Kanze Jun 14 '11 at 08:08
6

Using delete this is a bad idea if one is not sure of the pitfalls & working around them.

Once you call delete this the object's destructor is going to be invoked and the dynamically allocated memory will be freed.

If the object was not allocated using new, it will be a Undefined behaviour.
If any of object's data members or virtual functions are accessed after delete this, the behaviour will be Undefined Behavior again.

Probably, It is best to avoid delete this given the above.

Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • This is simply wrong. Or rather, it applies to all use of `delete`. There's nothing special about `delete this`. – James Kanze Jun 14 '11 at 08:09
4

It's actually a frequent idiom, and about as safe as any delete. As with all deletes, you have to ensure that no further code tries to access the object, and you have to be sure that the object was dynamically allocated. Typically, however, the latter is not a problem, since the idiom is only relevant for objects which have a lifetime determined by the semantics of the object, and such objects are always allocated dynamically. Finding all of the pointers too the object can be an issue (whether delete this is used or not); usually, some form of the observer pattern will be used to notify all interested parties that the object will cease to exist.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • I agree with Nawaz. The (current) top answers answer a different question, namely "When is it safe to delete this?", which is very different from "Ok, I can do this as long as I follow certain rules, but is it a good idea, or does it mean that I might be doing something else wrong?", which is a much more interesting question. – allyourcode Jul 18 '14 at 08:35
2

The idiomatic way of doing that in C++ is by putting the cleanup code in the destructor then let it be called automatically when you delete the object.

Class::~Class() {
    do_cleanup();
}

void ManagingClass::deleteNode(Class* instance) {
    delete instance; //here the destructor gets called and memory gets freed
}
GogaRieger
  • 345
  • 4
  • 10
  • How is this relevant to the question? The cleanup code is in the destructor. The question is who calls the destructor (via `delete`). – James Kanze Jun 14 '11 at 08:11
  • @James: in the question, there is some cleanup code in `Class::Delete`. It says so in a comment, "//Some cleanup code before deleting the object". I think it's reasonable to question why that cleanup *only* needs to happen when `Delete` is called, not for otherwise destruction of objects of type `Class`. As long as there's a good reason it's fine, of course. – Steve Jessop Jun 14 '11 at 09:35
  • @Steve Jessop It depends on what the function `Delete` really is; this is just an example. In real life, instead of `Delete`, it will be `HandleEventX`, or something like that, and it will be the only place `delete` will be called. Most of the time; you're right with regards to the general principle: any code which must be called whenever the object is destructed, regardless, belongs in the destructor, and not in the function which calls `delete`. – James Kanze Jun 14 '11 at 10:56
1

There's a simple way of doing the same thing that doesn't involve undefined behavior:

void Class::Delete() {
    //Some cleanup code before deleting the object
    std::auto_ptr delete_me(this);
}
Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
  • 2
    How is this different from `delete this`? I can't see how `auto_ptr` would boil down to anything but `delete this`, hence not have any advantages over the vanilla implementation. Am I wrong here? – larsmoa Jun 14 '11 at 06:28
  • 1
    This doesn't answer the question. Bad. – Nawaz Jun 14 '11 at 06:42
  • 1
    The only thing this does is add obfuscation. – James Kanze Jun 14 '11 at 08:10
  • 1
    @larsm, I just made an edit that should make the point clearer - the difference is that the `delete` won't happen until you leave the scope of the object. – Mark Ransom Jun 14 '11 at 12:14
  • 1
    @Nawaz, it does answer the question somewhat obliquely. If you have the choice of two techniques to do the same thing, one of which is guaranteed to be safe and the other which is not, you should prefer the safe one. – Mark Ransom Jun 14 '11 at 12:15