Stack Overflow when deleting this Pointer

Question

I have the following code and I get stack overflow error can anyone please explain me What's wrong here. from my understanding this pointer points to current object so why I cant delete it in a destructor;

class Object 
{
private:
    static int objCount;
public:
    int getCount()
    {
        int i =10;
        return i++;
    }
    Object()
    {
        cout<< "Obj Created = "<<++objCount<<endl;
        cout <<endl<<this->getCount()<<endl;
    }
    ~Object()
    {
        cout<<"Destructor Called\n"<<"Deleted Obj="<<objCount--<<endl;
        delete this;
    }
};

int Object::objCount = 0;

int _tmain(int argc, _TCHAR* argv[])
{


    {
        Object obj1;
    }

    {

              Object *obj2 = new Object();
    }
    getchar();
    return 0;
}

Answer 1

You're doing delete this; in your class's destructor.

Well, delete calls the class's destructor.

You're doing delete this; in your class's destructor.

...

<!<!<!Stack Overflow!>!>!>

_{(Sorry guys I feel obliged to include this... this might probably spoil it sorrrryyyy!}

Moral of the boring story, don't do delete this; on your destructor (or don't do it at all!)

Do ^[1]

Object *obj = new Object();
delete obj;

or much better, just

Object obj;

^[1]_{@kfsone's answer more accurately points out that the stack overflow was actually triggered by obj1's destructor.}

Answer 2

'delete this' never makes sense. Either you're causing an infinite recursion, as here, or you're deleting an object while it is still in use by somebody else. Just remove it. The object is already being deleted: that's why you're in the destructor.

Answer 3

The crash you are having is because of the following statement:

{
    Object obj1;
}

This allocates an instance of "Object" on the stack. The scope you created it in ends, so the object goes out of scope, so the destructor (Object::~Object) is invoked.

{
    Object obj1;
    // automatic
    obj1.~Object();
}

This means that the next instruction the application will encounter is

delete this;

There are two problems right here:

1. delete calls the object's destructor, so your destructor indirectly calls your destructor which indirectly calls your destructor which ...
2. After the destructor call, delete attempts to return the object's memory to the place where new obtains it from.

By contrast

{

          Object *obj2 = new Object();
}

This creates a stack variable, obj2 which is a pointer. It allocates memory on the heap to store an instance of Object, calls it's default constructor, and stores the address of the new instance in obj2.

Then obj2 goes out of scope and nothing happens. The Object is not released, nor is it's destructor called: C++ does not have automatic garbage collection and does not do anything special when a pointer goes out of scope - it certainly doesn't release the memory.

This is a memory leak.

Rule of thumb: delete calls should be matched with new calls, delete [] with new []. In particular, try to keep new and delete in matching zones of authority. The following is an example of mismatched ownership/authority/responsibility:

auto* x = xFactory();
delete x;

Likewise

auto* y = new Object;
y->makeItStop();

Instead you should prefer

// If you require a function call to allocate it, match a function to release it.
auto* x = xFactory();
xTerminate(x); // ok, I just chose the name for humor value, Dr Who fan.

// If you have to allocate it yourself, you should be responsible for releasing it.
auto* y = new Object;
delete y;

C++ has container classes that will manage object lifetime of pointers for you, see std::shared_ptr, std::unique_ptr.

Answer 4

There are two issues here:

1. You are using delete, which is generally a code smell
2. You are using delete this, which has several issues

---

Guideline: You should not use new and delete.

Rationale:

• using delete explicitly instead of relying on smart pointers (and automatic cleanup in general) is brittle, not only is the ownership of a raw pointer unclear (are you sure you should be deleting it ?) but it is also unclear whether you actually call delete on every single codepath that needs it, especially in the presence of exceptions => do your sanity (and that of your fellows) a favor, don't use it.

• using new is also error-prone. First of all, are you sure you need to allocate memory on the heap ? C++ allows to allocate on the stack and the C++ Standard Library has containers (vector, map, ...) so the actual instances where dynamic allocation is necessary are few and far between. Furthermore, as mentioned, if you ever reach for dynamic allocation you should be using smart pointers; in order to avoid subtle order of execution issues it is recommend you use factory functions: make_shared and make_unique (1) to build said smart pointers.

  (1) make_unique is not available in C++11, only in C++14, it can trivially be implemented though (using new, of course :p)

---

Guideline: You shall not use delete this.

Rationale:

Using delete this means, quite literally, sawing off the branch you are sitting on.

• The argument to delete should always be a dynamically allocated pointer; therefore should you inadvertently allocate an instance of the object on the stack you are most likely to crash the program.
• The execution of the method continues past this statement, for example destructors of local objects will be executed. This is like walking on the ghost of the object, don't look down!
• Should a method containing this statement throw an exception or report an error, it is difficult to appraise whether the object was successfully destroyed or not; and trying again is not an option.

I have seen several example of usage, but none that could not have used a traditional alternative instead.