15

Possible Duplicate:
C++: Delete this?
Object-Oriented Suicide or delete this;

I wonder if the code below is run safely:

#include <iostream>
using namespace std;

class A
{
public:
    A() {
        cout << "Constructor" << endl;
    }
    ~A() {
        cout << "Destructor" << endl;
    }

    void deleteMe() {
        delete this;
        cout << "I was deleted" << endl;
    }
};

int main()
{
    A *a = new A();
    a->deleteMe();
    cout << "Exit ...";
    return 0;
}

Output is:

Constructor
Destructor
I was deleted
Exit ...

and program exit normally, but are there some memory access violents here?

Community
  • 1
  • 1
Bình Nguyên
  • 2,252
  • 6
  • 32
  • 47
  • 1
    Related: http://stackoverflow.com/questions/3150942/c-delete-this – Fred Foo Aug 01 '12 at 09:52
  • 1
    Summary: no problem here, as long as you are careful (don't `delete` things that were not allocated with `new`, don't use an object after it has been destructed). – Jon Aug 01 '12 at 09:54
  • @RobertoWilko: My code is formatted with Visual Studio + Visual Assisstant, i don't know if this wrong :-( – Bình Nguyên Aug 01 '12 at 10:03

5 Answers5

9

It's ok to delete this in case no one will use the object after that call. And in case the object was allocated on a heap of course

For example cocos2d-x game engine does so. It uses the same memory management scheme as Objective-C and here is a method of base object:

void CCObject::release(void)
{
    CCAssert(m_uReference > 0, "reference count should greater than 0");
    --m_uReference;

    if (m_uReference == 0)
    {
        delete this;
    }
}

I don't think it's a c++ way of managing memory, but it's possible

Andrew
  • 24,218
  • 13
  • 61
  • 90
  • If I remember correctly that manner is used also in DirectX library. When you want to delete an object and free its memory you call SomeObj::Release() which works in similar way as the code you've posted. – tobi Aug 01 '12 at 10:00
  • So is the line `cout << "I was deleted" << endl;` safely here? – Bình Nguyên Aug 01 '12 at 10:00
  • 2
    @BìnhNguyên: I think yes, because you don't use the object, but I would prefere to print the message befoe `delete this` in any case – Andrew Aug 01 '12 at 10:02
  • 2
    The same pattern is used in COM's `IUnknown::Release()`, see http://msdn.microsoft.com/en-us/library/office/cc839627.aspx . – Sjoerd Aug 01 '12 at 10:12
  • "I don't think it's a C++ way of managing memory": it depends on the role of the object in the application (and on the type of application). For many OO oriented applications, it will be the usual way objects get deleted. (And I agree with Andrew's comment concerning doing the print before the delete.) – James Kanze Aug 01 '12 at 10:15
  • @JamesKanze: Yeah, possibly you are right, but I would at least write some strong_ptr or something like that to automate memory management in any case – Andrew Aug 01 '12 at 10:18
  • @Andrew The only pointer concerned with memory management in these cases is `this`, and the language doesn't allow you to make it a smart pointer. In the past, I've worked on an application which did use smart pointers to the object, which registered themselves with the object, and were automatically nulled in its destructor. In practice, it's usually not enough, since the pointer itself is still left floating around in maps, etc. You generally have to use the observer pattern so that everyone who holds a pointer to the object can do the right thing. – James Kanze Aug 01 '12 at 11:32
  • @JamesKanze: I ment some kind of scoped_ptr, which calls retain() in constructor and release() in destructor. It is possible and i did it – Andrew Aug 01 '12 at 12:00
  • @Andrew It's certainly possible (calling release in some `deleteMe` function, however, and not in the destructor). But it introduces additional complexity, for what? – James Kanze Aug 01 '12 at 15:09
  • @JamesKanze: This introduce no more complexity than any other smart pointer and frees you from remembering to call release()/retain() – Andrew Aug 01 '12 at 15:11
  • @Andrew "no more complexity than any other smart pointer": but `this` isn't a smart pointer, and you need nothing more. – James Kanze Aug 01 '12 at 15:21
  • @JamesKanze: http://en.wikipedia.org/wiki/Smart_pointer – Andrew Aug 01 '12 at 15:24
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/14750/discussion-between-andrew-and-james-kanze) – Andrew Aug 01 '12 at 15:28
  • @Andrew The Wiki page you cite is very poor, and extremely inaccurate. (For starters, smart pointers don't always have anything to do with memory management, and for another, Java has smart pointers (called WeakReferences) in its standard library.) – James Kanze Aug 01 '12 at 17:49
  • @JamesKanze: It's not said it always about memory management. Read the first sentence carefully – Andrew Aug 01 '12 at 18:38
1

It's ok, because you have running simple method. After delete this, all variables and virtual table are clear. Just, analyze this example:

#include <iostream>

class foo
{
public:
    int m_var;
    foo() : m_var(1)  
    {

    }

    void deleteMe()
    {
        std::cout << m_var << "\n";
        delete this;
        std::cout << m_var << "\n"; // this may be crush program, but on my machine print "trash" value, 64362346 - for example
    }

    virtual void bar()
    {
        std::cout << "virtual bar()\n";
    }



  void anotherSimpleMethod()
    {
       std::cout << "anotherSimpleMethod\n";
    }

    void deleteMe2()
    {
        bar();
        delete this;
        anotherSimpleMethod();
        // bar(); // if you uncomment this, program was crashed, because virtual table was deleted
    }
};

int main()
{
    foo * p = new foo();
    p->deleteMe();
    p = new foo();
    p->deleteMe2();
    return 0;
}

I've can't explain more details because it's need some knowledge about storing class and methods in RAM after program is loaded.

Torsten
  • 21,726
  • 5
  • 24
  • 31
0

Absolutelly, you just run destructor. Methods does not belongs to object, so it runs ok. In external context object (*a) will be destroyed.

Danil Speransky
  • 29,891
  • 5
  • 68
  • 79
0

When in doubt whether theres strange things going on in terms of memory usage (or similar issues) rely on the proper analysis tools to assist you in clarifying the situation.

For example use valgrind or a similar program to check whether there are memory leaks or similar problems the compiler can hardly help you with.

While every tool has its limitations, oftentimes some valuable insights can be obtained by using it.

mtd
  • 123
  • 7
-1

There is no memory access violation in it, you just need to be careful. But deleting this pointer is not recommended at all in any language, even though the code above would work fine. As it is same as delete a, But try doing it other way, the safe way.

For example there is something illogical in your posted code itself.

void deleteMe() 
{
    delete this;
    cout << "I was deleted" << endl; // The statement here doesn't make any sense as you no longer require the service of object a after the delete operation 
}

EDIT: For Sjoerd

Doing it this way make more sense

 void deleteMe() 
{
    delete this;
}

 int main()
 {
 A *a = new A();
 a->deleteMe();
 cout << "a was deleted" << endl;
 cout << "Exit ...";
 return 0;
 }

The second line in your deleteMe() function should never be reached, but its getting invoked. Don't you think that its against the language's philosophy?

ScarCode
  • 3,074
  • 3
  • 19
  • 32
  • -1: Pure nonsense about "this line should never be reached." – Sjoerd Aug 01 '12 at 10:05
  • You don't get the whole picture. Of course it gets executed. But go through my post once again, you may get it then:D – ScarCode Aug 01 '12 at 10:09
  • You contradict yourself: "this line should never be reached," but "of course it gets executed." Are you suggesting that it is a compiler bug?! – Sjoerd Aug 01 '12 at 10:16
  • And what do you mean with "ofc it gets executed **as function body is placed in main**"? Why should `main()` be special in this case? Secondly, the function body is defined inline in the class definition, and not placed in main. If you mean that it might get inlined, that's true, but inlining or not has nothing to do with wether this code is safe. – Sjoerd Aug 01 '12 at 10:20
  • Ah! deleteMe() is a member function of Object a, So it should be invoked by object a. But by doing "delete this" before that cout statement, the object that invokes its member function is no more and so should its member functions. So that cout statement is getting executed after the death of object a. That doesn't make any sense at all even though there is no errors in doing so. That's my point. Don't always be technical. – ScarCode Aug 01 '12 at 10:48
  • You are using a lot of "shoulds" that are unsupported by the C++ standard. Yes, object a does not exist anymore at the point of the cout, but that doesn't stop the function from executing. It's just not possible to use `this` anymore, which includes calling virtual functions. As long as you are very carefully, it is possible to execute some code after the `delete this;` and before the `return;` statement. However, to be really sure `this` is never used in that part, experts recommend to put everything *before* the `delete this;` statement. – Sjoerd Aug 01 '12 at 10:55
  • WRT "Don't always be technical": When the question is whether it is **technically allowed**, one should get technically in the answer. – Sjoerd Aug 01 '12 at 10:57
  • But as it is "Technically" allowed and considering I mentioned it, You should try coming the other way around. What I was just trying to say was that ain't a good programming practice. – ScarCode Aug 01 '12 at 11:01
  • 1
    Re: "the object that invokes its member function is no more and so should its member functions" - that's complete nonsense. An object's non-static data is completely separate from any code that accesses it (including member functions), and destroying an object only destroys that data. The code still exists, is still executable, and is still well-defined as long as it doesn't access the destroyed object. – Mike Seymour Aug 01 '12 at 11:13