Why is the destructor not called for stack variable when using assignement?

Question

This stupid code snipped already took my 2 hours, I cannot figure out why the destructor of the first element, the one with size 7, not called? What happens to the memory allocated for new uint16_t[7]?

#include <iostream>

using namespace std;

struct Node
{
    Node(uint16_t n) : p(new uint16_t[n]) {
        cout<<"Constructed with size= "<<n<<", memory addr: "<<(p)<<endl;
        for(uint16_t i=0; i<n; i++) p[i] = n;
    }

    ~Node() {
        cout<<"Destructor for p[0] = "<< *p <<" with memory addr: "<<p<<endl;
        delete[] p;
    }

    uint16_t *p;
};

int main()
{
    {
        Node nd1(7);
        {
            nd1 = Node(3);
            cout << "1st place holder" << endl;
        }
        cout << "2nd place holder" << endl;
    }

    return 0;
}

The output is

Constructed with size= 7, memory addr: 0x158cc20                                                                                                                                   
Constructed with size= 3, memory addr: 0x158cc40                                                                                                                                   
Destructor for p[0] = 3 with memory addr: 0x158cc40                                                                                                                                
1st place holder                                                                                                                                                                   
2nd place holder                                                                                                                                                                   
Destructor for p[0] = 0 with memory addr: 0x158cc40                                                                                                                                
*** Error in `./a.out': double free or corruption (fasttop): 0x000000000158cc40 ***                                                                                                
Aborted (core dumped)

The double free is because your class violates the rule of 3 /5 /0: [https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — drescherjm, Jan 23 '20 at 21:07
The builtin operator=() will do a member wise copy of your class which ends up duplicating value the pointer. — drescherjm, Jan 23 '20 at 21:10
@drescherjm Indeed what I expected is when I use = operator, at first the destructor for Node(7) is called, and then the assignment happens. But apparently not. — crbah, Jan 23 '20 at 21:12

eerorika · Answer 1 · 2020-01-23T21:17:27.003

I cannot figure out why the destructor of the first element, the one with size 7, not called?

It is called. In fact, it is that destructor which causes the program to crash. The behaviour of the program is undefined because the destructor deletes the same pointer value that had previously been deleted by the destructor of the temporary object. And also before that it indirects through that invalid pointer.

What happens to the memory allocated for new uint16_t[7]?

The pointer to the memory was lost when you assigned over the object. This is called a memory leak.

Guillaume Racicot · Accepted Answer · 2020-01-23T21:40:35.147

This code: nd1 = Node(3); does not what you expect.

It does not replace nd1 with Node(3) and kill the old nd1.

What it does is it creates a new temporary node instance Node(3) and copy the value of every members into nd1. So both the temporary and nd1 contain a pointer that points to the same address. At that point, you leaked the memory nd1 allocated at the start of the program since no pointer is refering to it but you didn't deleted it.

When the temporary dies, nd1 points to dead memory. When nd1 runs its destructor at the second }, it delete the same pointer again hence your error.

To fix this, you'll have to implement what is called the rule of five or the rule of zero.

The easiest is the rule of zero. Simply use unique_ptr and the destruction happens as expected:

struct Node
{
    Node(uint16_t n) : p(std::make_unique<uint16_t[]>(n)) {
        cout<<"Constructed with size= "<<n<<", memory addr: "<<(p)<<endl;
        for(uint16_t i=0; i<n; i++) p[i] = n;
    }

    std::unique_ptr<uint16_t[]> p;
};

int main()
{
    {
        Node nd1(7);
        {
            nd1 = Node(3); // assignement destroys the old buffer
            cout << "1st place holder" << endl;
        }
        cout << "2nd place holder" << endl;
    }

    return 0;
}

"This code: nd1 = Node(3); does not what you expect."--> Exactly. I expected a sequence of operations where at first we need to get rid of the existing value. And then make a new assignment. — crbah, Jan 23 '20 at 21:14
You forgot to remove the now superfluous `delete[] p;` from the destructor. Also, for C++14 or higher, it's safer (for potentially more complicated code with other state to initialize) to change the initializer from `p(new uint16_t[n])` to `p(std::make_unique(n))` as it guarantees execution occurs in an order such that exceptions in unsequenced code near it can't occur after the allocation, but before the `unique_ptr` is initialized, causing a leak. — ShadowRanger, Jan 23 '20 at 21:36

Why is the destructor not called for stack variable when using assignement?

2 Answers2