🔒 Locked

Deleting an Object that is being used

Started by aeroz Jun 21, 2008 at 2:48 PM 55 replies 7.8k views

Original Post

Jun 21, 2008 at 2:48 PM

I have a general C plus plus question: What does happen when you are inside an object's method and the the object is deleted? Example: (corrected)

class Abc;

class Obj
{
public:
   void DoSomething()
   {
      Abc->DeleteObj();
      /* What now? */
   }
   Abc* blaBla;
};

class Abc
{
public:
   Obj* obj;
   void DeleteObj() { delete obj; }
};

int main()
{
   Obj obj1;
   Abc abc;

   abc.obj = &obj1
   obj1.blaBla = &abc
   obj1.DoSomething();
}

I know it's not a good example since both classes have pointers to each other. But what would happen with the "obj1" object? Would the destructor be called? Would the DoSomething() function continue executing? Thank you for the help! [Edited by - aeroz on June 21, 2008 4:16:29 PM]

Mike nl

Jun 21, 2008 at 2:57 PM

Quote:
Original post by aeroz
I know it's not a good example since both classes have pointers to each other.

Well, one has, but I'll assume the other is a typo and should be a pointer too ;) In main() you should also have &obj1 to assign to the pointer.

Quote:
Original post by aeroz
But what would happen with the "obj1" object? Would the destructor be called?

Yes, an implicit one, because you have none defined. Worse though, it's memory is freed after destruction which is a Bad Thing because it was allocated on the stack of main().

Quote:
Original post by aerozWould the DoSomething() function continue executing?

In this case, no, because of the stack issue. However, in the following, proper example it would continue just fine:

class Abc;class Obj{public:   Abc* blaBla;   void DoSomething()   {      blaBla->DeleteObj();   }};class Abc{public:   Obj* obj;   void DeleteObj() { delete obj; }};int main(){   Obj* obj1 = new Obj;   Abc abc;   abc.obj = obj1;   obj1->blaBla = &abc   obj1->DoSomething();}

I'm not sure if the standard guarantees this behavior to be well defined, but practically it'll work fine.
AFAIK, you only get undefined behavior if you try to use members variables or (virtual) functions of the deleted object in DoSomething() after calling DeleteObj().

RealMarkP

Jun 21, 2008 at 2:58 PM

This is just bad design. It would be the same as trying to delete the 'this' keyword:

void someClass::someFunciton(){  delete this;  // What happens now??? What is 'this'?? What am I acting on?}

Firstly, after that delete command, you will be acting upon memory that has been deleted, essentially causing a "buffer overflow" or worse, a heap corruption. If you can, try to rework your logic.

Mike nl

Jun 21, 2008 at 3:01 PM

Quote:
Original post by RealMarkP
This is just bad design. It would be the same as trying to delete the 'this' keyword:

There's nothing wrong with delete this; I use it in reference counted objects:

void Object::Release() {    if (--m_nReferences == 0) {        delete this;    }}

Combine this with private destructors, and you enforce referencing counting.

coderx75

Jun 21, 2008 at 3:04 PM

Obj contains an Abc object but Abc contains a pointer to an Obj object. So, it's basically a chain of objects. If one instance of Abc deletes its Obj pointer, it simply breaks the chain (probably causing a memory leak). You're not actually deleting an object "in use".

If you're finding yourself stuck in a circular dependency such as this, you probably need to seriously rethink your code. As for deleting object while in use, you may want to consider delete "events" instead. This allows other parts of the app to communicate that it wishes to delete while the actual deletion is held off until the object is not in use.

Hope this helps.

fpsgamer

Jun 21, 2008 at 3:19 PM

Quote:
Original post by Mike nl
Quote:
Original post by RealMarkP
This is just bad design. It would be the same as trying to delete the 'this' keyword:

There's nothing wrong with delete this; I use it in reference counted objects

Almost, but I think you get the idea.

@OP, could you post your actual interface. It would be easier to make suggestions if we knew the task at hand.

aeroz

Jun 21, 2008 at 4:32 PM

Thank you for your answers!

Quote:
Well, one has, but I'll assume the other is a typo and should be a pointer too ;) In main() you should also have &obj1 to assign to the pointer.

Yes, my mistake! (I've corrected it now.)

Quote:
AFAIK, you only get undefined behavior if you try to use members variables or (virtual) functions of the deleted object in DoSomething() after calling DeleteObj().

That's exactly what I think, but I don't know if it's true. (platform/compiler dependent?)

Quote:
Yes, an implicit one, because you have none defined. Worse though, it's memory is freed after destruction which is a Bad Thing because it was allocated on the stack of main().

Quote:
Original post by aerozWould the DoSomething() function continue executing?

In this case, no, because of the stack issue. However, in the following, proper example it would continue just fine:

Mike nl: Yes, I agree. I also have dynamically allocated objects. But is it really true that in your example the DoSomething() method would continue running?

Actually I came across this problem while reading this article: Game States
In the Intro state itself the state is being changed (that means that the Intro state is being deleted). Here the code:

void CIntroState::HandleEvents(CGameEngine* game){	/* .... code 1 */        if ( /* key to change state is pressed */ )	      game->ChangeState( CPlayState::Instance() );        /* .... code 2 */}

In ChangeState() the current state (Intro) is deleted and a new one is initialized (CPlayState). Will code 2 be executed?

janta

Jun 21, 2008 at 5:14 PM

@fpsgamer

Question regarding your link: (item #4)

Quote:
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.

I don't understand why it would be wrong to do something with "this" it self afeter "delete this" as long as such operation doesn't involve dereferencing "this"

printf("%x", this)if (this > 0x80000000)int* p = (int*) this;

and so on. Can delete have side effects on the pointer itself?

fpsgamer

Jun 21, 2008 at 5:37 PM

Quote:
Original post by janta
@fpsgamer

Question regarding your link: (item #4)

Quote:
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.

I don't understand why it would be wrong to do something with "this" it self afeter "delete this" as long as such operation doesn't involve dereferencing "this"
printf("%x", this)if (this > 0x80000000)int* p = (int*) this;
and so on. Can delete have side effects on the pointer itself?

Well you just deleted this, what obligation does the compiler have to keep the this pointer around?

It would be no more valid than doing this:

class Foo{    int* myIntPtr;public:    ...    void freeFoo()    {         delete this;         /* I'm not dereferencing these pointers,             but the following is still bad:          */         assert(myIntPtr != 0);          assert(this != 0);    }};

Once you delete the object, everything having to do with this is kaput, including the this pointer.

Mike nl

Jun 21, 2008 at 6:03 PM

Actually, that really means:

delete this;
assert(this->myIntPtr != 0);
assert(this != 0);

So in the first assert, you are dereferencing a deleted pointer, so I don't think that argument holds. However, I'm not sure what does happen to this, so yes, I'd also recommend against doing anything with it.

Practically, I think nothing happens with the pointer itself, just like any other pointer that you delete. However, because you can't dereference it, it's kind of useless after the delete.

Then again, C++ has weird stuff going on with pointers and multiple inheritance and virtual stuff, so I can't completely rule out nothing funky will happen. Just don't use a pointer after deleting it, is generally sound advice ;)

aeroz

Jun 22, 2008 at 3:09 AM

So, is it safe on EVERY system to do this?

class CObj{   DeleteMe() { delete this; }   ~CObj() { /* will everything here be called? */ }};int main(){   CObj* ptr = new CObj;   ptr->DeleteMe();   return 0;}

[Edited by - aeroz on June 22, 2008 5:09:10 AM]

Gage64

Jun 22, 2008 at 3:19 AM

Quote:
Original post by aeroz
So, is it safe on EVERY system to do this?

*** Source Snippet Removed ***

Not really.

janta

Jun 22, 2008 at 3:45 AM

Quote:
Original post by fpsgamer

It would be no more valid than doing this:

*** Source Snippet Removed ***

Once you delete the object, everything having to do with this is kaput, including the this pointer.

Yes, you are dereferencing "this" in the following operation

assert(myIntPtr != 0);

because it is equivalent to

assert(this->myIntPtr != 0);

which is indeed very bad, but I don't see how

delete this;assert(this != 0);

is any worse than

int* p = new int [32]delete [] p;assert(p != 0);

which is OK afaik (I could be wrong though)

aeroz

Jun 22, 2008 at 5:10 AM

@Gage64:
In the link you posted there nothing against my code?? In [16.15] it just says you mustn't play around with the this pointer when you have deleted it.

@janta:

Quote:
int* p = new int [32]
delete [] p;
assert(p != 0);

which is OK afaik (I could be wrong though)

I don't think this is OK since you always have to set a pointer to NULL when you delete it, isn't it? At least I thought so. p would still be the address of the data that doesn't exist anymore.

int* p = new int;
delete p;
p = NULL;
assert(p != 0); // Thats obvious now...

I'm not sure if this also applies to arrays:

int* p = new int [32];
delete [] p;
p = NULL;
assert(p != 0);

Gage64

Jun 22, 2008 at 6:25 AM

Quote:
Original post by aeroz
@Gage64:
In the link you posted there nothing against my code?? In [16.15] it just says you mustn't play around with the this pointer when you have deleted it.

What I meant to say was that it is easy to mess up when doing this, and that link lists several reasons why.

Another reason is that the programmer using your class must know not to delete it even though he new'd it, which is unintuitive and goes against usual programming practice.

alvaro

Jun 22, 2008 at 7:26 AM

Quote:
Original post by aeroz
So, is it safe on EVERY system to do this?

*** Source Snippet Removed ***

That depends on your definition of "safe". If I were your boss, I might consider firing you if you write that type of code, so it's not that "safe", is it? ;)

Whenever you do something that restricts the way your class can be used, you should write a clear comment describing these restrictions, in a place where your class's users won't miss it.

aeroz

Jun 22, 2008 at 8:14 AM

OK, thank you all!

fpsgamer

Jun 22, 2008 at 11:30 AM

Quote:
Original post by janta
Yes, you are dereferencing "this" in the following operation
assert(myIntPtr != 0); 
because it is equivalent to
assert(this->myIntPtr != 0); 

You're quite right. In my haste I accidentally produced a flawed example.

Quote:
Original post by janta
you always have to set a pointer to NULL when you delete it, isn't it?

You don't have to, its just something people like to do.

Quote:
Original post by janta
which is indeed very bad, but I don't see how
delete this;assert(this != 0);
is any worse than
int* p = new int [32]delete [] p;assert(p != 0);
which is OK afaik (I could be wrong though)

This is okay:

{int* p = new int[32];delete [] p;assert(p != k); // this is o.k.}

Since you allocated p, you own it and do whatever you please with it within the block scope where p is guaranteed to still exist.

However this is not okay:

delete this;assert(this != k); // this is bad

You don't own the this pointer. The this pointer is only guaranteed to exist for as long as the object exists. Once the object is deleted, the this pointer and everything else associated with the object is invalid to use.

janta

Jun 22, 2008 at 2:48 PM

Quote:
Original post by fpsgamer
However this is not okay:
delete this;assert(this != k); // this is bad
You don't own the this pointer. The this pointer is only guaranteed to exist for as long as the object exists. Once the object is deleted, the this pointer and everything else associated with the object is invalid to use.

Thanks!

Here\s a basic test I did:

struct T { T() { int i = this; } }; // cannot convert from 'T *const ' to 'int'

So I thought in the context of a member function, 'this' was constant. I guess as you said the standard does not enforce this, though in my own experience I've never seen the value of 'this' changng in the middle of a function.

Mike nl

Jun 22, 2008 at 3:07 PM

I'm not sure what you want to show with that test. The error is about a pointer-to-int conversion which has nothing to do with "this".

But anyway, in the C++ standard, section 9.3.2 "The this pointer" they state (emphasis mine):

Quote:
In the body of a nonstatic (9.3) member function, the keyword this is a non-lvalue expression whose value is the address of the object for which the function is called.

Although I couldn't find any mention of using this after deleting the object, I think it's safe to say that since there is no object, there can't be an address to it either. So using this (even in a pointer comparison) after deleting an object would probably be undefined behavior, strictly speaking.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.