Assertion failure: calling a list's node deconstructor

Question

I am a computer science student with the task of creating a dynamic data structure, linked lists. I am currently working on a singly linked list and have successfully built the functionality to add, remove and dump all node data.

However, remembering that my 'advanced programming' lecturer stated that in order to avoid confusion and other problems, when deleting nodes from a list, or releasing any object's memory, you should have it happen inside its deconstructor. So I moved:

delete[] _del;

Which worked fine and moved it to the nodes' deconstructor:

#include "Node.h"

// Node.cpp

Node::Node(const int &inData, const int &inId)
{
    _id = inId;
    _data = inData;
    nextNode = NULL;
}

// Deconstructor to delete the node when using List.Del()
Node::~Node()
{
    delete[] this;
}

In my List the node's deconstructor is called via a pointer like so:

_del->~Node();

Which gives me an assertion error. I'm assuming it is my usage of 'this' in the node's deconstructor?

Thanks for your time.

Answer 1

First of all, you should not call an objects destructor directly, unless you're writing an allocator and used placement new when creating it. Second, you should delete and not delete[] unless you also used new[]. And finally, delete this is a bad habit, but legal according to the standard. Why don't you just call delete theNode instead of all of this?

EDIT: Addressing some comments/additional questions.

To allocate a single instance on heap, you use theNode = new Node. The returned pointer must be freed with delete theNode. Calling new will allocate memory and then call Node::Node(), the constructor, so that it can setup it's internal state. Calling delete will call Node::~Node() and then free the allocated memory. The destructor is responsible for cleaning up anything Node uses, but not the memory used by Node itself.

To allocate an array of nodes, you use theNodes = new Node[10];. You delete these with delete[] theNodes. Mixing new/delete with new[]/delete[] is undefined behaviour.

Placement new is a method where you want to construct an object in already allocated memory. In this case, you have the only good reason for calling a destructor directly, you want to deconstruct an object (aka letting it clean itself up) without also freeing the memory allocated for it.

Calling delete this is legal in e.g. a Suicide() function, as long as you do not refer to "this" or any members of the deleted instance after the call to delete this. This is a valid technique e.g. in reference counted objects, but is often considered something you should avoid unless you really need it.

The correct solution for you is pretty plain, where you now call ~Node, simply call delete theNode instead.

Answer 2

How is an object deleted from within its deconstructor, correctly?

It's not. A destructor cleans up the resources owned by an object, not the object itself.

You should remove the use of delete and whatever allocated the Node (your list class?) had better be responsible to deallocate it. The easiest way to do this is have exactly one point in your list class that allocates and adds nodes to the list, and exactly one point that removes and deallocates nodes (the mentioned Del method). Have your list class' destructor call Del repeatedly until the list is empty.

Answer 3

Node::~Node()
{
    delete[] this;
}

Undefined behavior. Most likely will crash your program!

By the way, there is a difference between delete this and delete[] this. While delete this might be okay sometime, but delete[] this is not, as this can never be allocated using new[]. It's not a pointer to an array. It's a pointer to ONE object!

Answer 4

As a previous answer suggests your deconstructor is undefined behaviour. For the following reason:

When

delete[] _del;

is executed. it attempts to delete the same object, which you are trying to delete in its deconstructor

delete[] this;

What your course text means is that, IF you want to clear up any memory associated tothe object being deleted that should be released in this object's deconstructor as well. For example if id and/or data were pointers:

Node::~Node()
{
   delete _id;
   delete _data;
}