"Half-way house" deletion in destructor and attempting to remove references in other objects

Question

I have a tree structure, in which every node contains a pointer to its parent object and a vector of its child objects. My intention is that when a node gets deleted, it deletes its children, which in turn delete their children, etc.

Compartment::Compartment(int inpID, eType inpEnum, double inpX, double inpY, double inpZ, double inpR, Compartment* inpParent){
    ID = inpID;
    ...
    parent = inpParent;
    std::vector<Compartment*> v;
    children = v;
    if (parent != nullptr){
        parent->children.push_back(this) //Is this poor coding?
    }
}

Compartment::~Compartment(){
        /*int pos;
        for (int ii = 0; ii < getParent()->getChildren().size; ii++){
            if (getParent()->getChildren()[ii]->getID() == ID){
                pos = ii;
                getParent()->getChildren().erase(getParent->getChildren().begin()+pos);
            }
        }*/    //Un-commenting this gives a double-free error
    std::cout << "Deleting " << ID << "\n";
    for (int ii = 0; ii < children.size(); ii++){
        delete children[ii];
    }
}

I also wanted to get it to delete itself from its parents vector of child nodes, but while trying to debug that (it caused a double-free error) I found some behaviour in the destructor I couldn't explain, including different "amounts" of deletion.

The code I used for testing this is here:

int main(){                           //ID type  co-ordinates    parent
    Compartment *root = new Compartment(0,ENUM_A,0.0,0.0,0.0,1.0,nullptr);
    Compartment *leaf = new Compartment(1,ENUM_A,1.0,2.0,2.0,1.0,root);
    Compartment *leaf2 = new Compartment(2,ENUM_A,1.0,2.0,2.0,1.0,root);
    Compartment *leaf3 = new Compartment(3,ENUM_A,1.0,2.0,2.0,1.0,leaf);
    std::cout << "Children of root:\n";
    std::vector<Compartment*> kids = root->getChildren();
    for (int ii = 0; ii < kids.size(); ii++){
        std::cout << "ID No. " << kids[ii]->getID() << "\n";
    }
    std::cout << "Children of leaf:\n";
    kids = leaf->getChildren();
    for (int ii = 0; ii < kids.size(); ii++){
        std::cout << "ID No. " << kids[ii]->getID() << "\n";
    }
    std::cout << "Deleting leaf\n";
    delete leaf;
    std::cout << "Children of root:\n";
    kids = root->getChildren();
    for (int ii = 0; ii < kids.size(); ii++){
        std::cout << "ID No. " << kids[ii]->getID() << "\n";
    }
    std::cout << "ID of leaf: " << leaf->getID() << "\n";
    std::cout << "ID of leaf3: " << leaf3->getID() << "\n";
}

What I get when I run this isn't what I expect, and I can't quite explain it:

Children of root:
ID No. 1
ID No. 2
Children of leaf:
ID No. 3

This is all as expected.

Deleting leaf
Deleting 1
Deleting 3
Children of root:
ID No. 28168496
ID No. 2

OK, so this is just looking at freed memory

ID of leaf: 28168496
ID of leaf3: 0

So, clearly leaf3 hasn't been deleted in the same way as leaf. Its fields have been altered, even when accessed outside of the children vector, but I think the memory has not been freed? What's more, if I append another delete leaf3 to the program, it does so with no problem, leading to it behaving like leaf, but if I put add in delete leaf it gets stuck in an infinite loop. This behaviour is consistent, and always the same way round: finding the ID of leaf returns the numbers, but leaf3 always yields 0.

What is happening here, and how would I properly go about deleting the children of a node? Is this related to my problem with removing the data from the vector?

Answer 1

The code looks correct. As long as you deleted the pointer, the object it pointed to has been destroyed and the memory has been made available for reuse. Don't poke around with the memory pointed to by invalid pointers like leaf3 after it's been deleted; there's nothing meaningful there. The memory manager may have done something to the contents, or it may not have.

Answer 2

In your code you write:

delete leaf;
leaf->getID();

This causes undefined behaviour. You cannot use leaf after deleting the object it is pointing to. Nor can you use leaf3.

Another problem is that you write:

delete leaf;

which leaves the root's list of children containing a dangling pointer. You need to remove this pointer from root's list before deleting it.

You can see some evidence of this in your program output: you get a garbage value as the first child's ID. This is actually undefined behaviour , anything could have happened.

You didn't show enough of your code to indicate whether a compartment also contains a pointer to its parent. If so, you could modify Compartment's destructor to remove itself from its parent when it is being deleted.

Otherwise, you will need to find some other way to remove a pointer from the structure before deleting it.

I append another delete leaf3

Deleting the same memory twice also causes undefined behaviour.

Consider making your compartments contain smart pointers instead of raw pointers, then you don't have to worry about any of this.