Thread: Object self-destruction

Hi there,

I've got two simple questions.

1. Is "delete this" safe (I'm reading it is!?), assumming method will return (void) immediately after deleting.

2. Is the following code safe:

Code:

void Layer::StrangeDelete()
{
    shared_ptr<Layer> lockme = this_ptr(); // lockme holds shared_ptr to this now
    _Parent->DeleteChild(); // delete original shared_ptr
    // lockme goes out of scope and there is no more strong references
    // "delete this" will take place, but it is no longer in the method scope, so is it safe or not?
}

Thanks

a guess..but i read that it is unsafe...
because you might call the destructor twice..well this I know for simple objects..maybe its the same concept?

You could read this FAQ: Is it legal (and moral) for a member function to say delete this?

Thanks, that helped.

Originally Posted by laserlight

You could read this FAQ: Is it legal (and moral) for a member function to say delete this?

I'm not quite sure why #4 is bad?

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.

De-referencing it would obviously be bad, but a pointer is just an integer of a memory address, so how can comparing it to NULL be harmful (albeit kind of useless, since delete doesn't set it to NULL)?

Originally Posted by cpjust

I'm not quite sure why #4 is bad?



De-referencing it would obviously be bad, but a pointer is just an integer of a memory address, so how can comparing it to NULL be harmful (albeit kind of useless, since delete doesn't set it to NULL)?

delete doesn't set a pointer to NULL, your pointer still points to that address. Delete just tells the computer that the memory is free to use. If that memory hasn't been used by the computer for other purposes yet, your old value still exists there, but there is no guarantee that. If you want your pointer to be NULL, you have to set it yourself After delete.

Originally Posted by nimitzhunter

delete doesn't set a pointer to NULL, your pointer still points to that address. Delete just tells the computer that the memory is free to use. If that memory hasn't been used by the computer for other purposes yet, your old value still exists there, but there is no guarantee that. If you want your pointer to be NULL, you have to set it yourself After delete.

I'm pretty sure cpjust understands that! The point being made was simply that such comparisons couldn't possibly be dangerous...

Yea, comparing and other operations seem to be safe, except casting, which may dereference 'this' (dynamic_cast). Well, calling other (non-virtual) methods should be (?) safe as well, unless they touch any member variables (despite of the fact that this is useless). And why anyone would need to compare this with NULL if it cannot be explicitly assigned...

Originally Posted by cpjust

De-referencing it would obviously be bad, but a pointer is just an integer of a memory address, so how can comparing it to NULL be harmful (albeit kind of useless, since delete doesn't set it to NULL)?

The "#4" rule is possibly being over-cautious, but the point is that "delete this" does not play nicely with a lot of techniques that are often used to give assurances that subsequent code is safe/bugfree/etc.

To understand "#4" picture a circumstance where a programmer has turned on debugging output while trying to find the cause of a spurious crash.

He or she comes across code like this .....

Code:

if (debugging)
   std::cerr << " Pointer p is " << (void *)p << '\n';
if (p != NULL)
   p->some_member_function();

On examining the output, it will be seen that p results in some (non-zero) address. This is immediately followed by a test that p is not NULL protecting the call of the member function. Ergo, our developer concludes the crash cannot be occurring here.

However, if p has self-destructed ("delete this"), then we have the situation of a programmer assuming code is safe when it is not. A lot of time can be wasted by a developer trying to track down a bug in code that falsely gives reassurance there is no bug.

Originally Posted by grumpy

The "#4" rule is possibly being over-cautious, but the point is that "delete this" does not play nicely with a lot of techniques that are often used to give assurances that subsequent code is safe/bugfree/etc.

To understand "#4" picture a circumstance where a programmer has turned on debugging output while trying to find the cause of a spurious crash.

He or she comes across code like this .....

Code:

if (debugging)
   std::cerr << " Pointer p is " << (void *)p << '\n';
if (p != NULL)
   p->some_member_function();

On examining the output, it will be seen that p results in some (non-zero) address. This is immediately followed by a test that p is not NULL protecting the call of the member function. Ergo, our developer concludes the crash cannot be occurring here.

However, if p has self-destructed ("delete this"), then we have the situation of a programmer assuming code is safe when it is not. A lot of time can be wasted by a developer trying to track down a bug in code that falsely gives reassurance there is no bug.

But that situation can happen whether the object self-deletes or whether you explicitly say 'delete p;' For NULL comparisons to be useful in either case you still have to set the pointer to NULL:

Code:

Obj* p = new Obj;
delete p;
p = NULL;

or

Code:

Obj* p = new Obj;
p->Delete();  // Calls delete this;
p = NULL;

Originally Posted by Sebastiani

I'm pretty sure cpjust understands that! The point being made was simply that such comparisons couldn't possibly be dangerous...

I've had this argument with folks before, and I'm pretty much squarely on your side. There's no imaginable physical architecture where simply "speaking the name" of invalid memory would cause a problem.

For one thing, no architecture I know of distinguishes between pointers and integers at the machine code level. So there is no way for the machine to even tell if a given value is a pointer in order to check if it's valid. Second, performing such a check every time a value transfers between registers or memory would be unrealistic.

I'd pay, oh... $250 to see a problem on any actual architecture.

On the other hand, you could have a code validator/analyzer like Valgrind that actually tracks this stuff and blows up when someone uses an invalid pointer value -- such as storing it into an array for later use -- but in that case, the explosion is exactly what you want because you're trying to FIND such problems.

Either way, it isn't something I'd lie awake worrying about.

Originally Posted by cpjust

But that situation can happen whether the object self-deletes or whether you explicitly say 'delete p;'

Sure. That's why I suggest the guideline is over-cautious. I described the type of rationale I've seen offered for that guideline. I did not say I agree with it.

The basic gist of the set of guidelines is valid, however. An object that deletes itself does tend to seriously break common working assumptions of calling code. Those four guidelines (even the last) basically make it difficult for calling code to assume an object exists after it has actually self-destructed.

Personally, I prefer objects get deleted by some other containing object, or managed using scope, rather than an object self-destructing.

I also never set a pointer to NULL after deleting it. Instead, I go out of my way to ensure the pointer also ceases to exist (eg by going out of scope) immediately after the object it points at is destroyed.

Originally Posted by grumpy

Personally, I prefer objects get deleted by some other containing object, or managed using scope, rather than an object self-destructing.

I also never set a pointer to NULL after deleting it. Instead, I go out of my way to ensure the pointer also ceases to exist (eg by going out of scope) immediately after the object it points at is destroyed.

Amen to that one!

In a code review the other day I caught someone using a SAFE_DELETE macro, which checked for NULL (yuck) and set to NULL (yuck) in addition to the delete. Oh and of course the only place it was called was in the destructor...

Originally Posted by grumpy

Personally, I prefer objects get deleted by some other containing object, or managed using scope, rather than an object self-destructing.

For the vast majority of pointers, I'd agree with that, but what about pointers to objects created inside DLL's? I don't think it's safe to delete a pointer that was allocated in another module like a DLL. That's why at a previous company they had a Delete() function in the classes returned by factory methods in a DLL.