[ Why does my destructor only crash my code after it's called by its parent destructor? ]

I have a doubly threaded linked list made up of winery elements, winery being a class with its own data fields. The problem that I was having was with winery's destructor:

winery::~winery()
{
        delete[] name;
        delete[] location;
}

I received a "double free or corruption (out): 0x0000000000402940" error in Linux, and I received this error in Windows Visual Studio:

So I put a breakpoint on my winery's destructor and found that it was being called every time I referenced my winery object. Strangely, the only time an error was thrown was when winery's destructor was called because my list's destructor had been called, even if winery had already been called two or three times. I would like to know why my code didn't crash the second time winery's destructor was called when it wasn't called by its parent destructor.

As a side note/question: I was able to solve this issue of memory leakage by removing winery's destructor and putting its calls in it parent destructor like this:

list::~list()
{
    for (node* next; headByName; headByName = next)
    {
        next = headByName->nextByName;
        delete[] headByName->item.getName();
        delete[] headByName->item.getLocation();
        delete headByName;
    }
}

winery is a type with name 'item' and getName() and getLocation() are functions of winery which return those cstrings. This works but it's counter-intuitive, and it certainly seems inefficient. Is it bad code? Should I be using `winery's destructor and overriding when it's called somehow?

Answer 1

---

Your code appears to be calling delete on some of the pointers multiple times.

delete[] headByName->item.getName();
delete[] headByName->item.getLocation();

and

delete[] name;
delete[] location;

I would suggest removing the first two lines.

Also, as suggested in the comments, you might be facing problems due to not implementing the copy constructor and the copy assignment operator in winery. Have a look at What is The Rule of Three? for more insight into that problem.

Answer 2

---

Your linked list implementation is flawed. You are storing winery and not a pointer to it. This means that without a proper deep copy constructor the data is not duplicated, only the pointer values. This will cause the destruction.

Either modify your linked list to store pointers (the better way) or provide a copy constructor that duplicates the data. The latter will cause more memory allocations and be slower, as well as not allow you to actually change the values so that every place the specific winery is stored would have the same value.

When not using pointers what happens is this:

{
    Winery foo("name"); // constructor copies name to a new buffer
    Winery bar = foo; // A new copy is created, shallow copy of data. This means bar.name = foo.name!
    ...
}
// automatic destruction of objects, both have the same pointers resulting in double free