c++ destructor de-allocation issue

Question

I'm trying to write a class that contains a double* array which can be filled via various ways, at the end of the program, the memory should be deallocated - alas, this does not work.

I get the notification that "programname.exe has triggered a breakpoint", which leads me to the last line of my main.cpp - when I delete my destructor it works fine, so I'm assuming it has got to do something with that.

Here's the relevant code:

.h

    #pragma once
using namespace std;
#include <iostream>
class polynom
{
public:
    polynom(int grad, double* arr);
    polynom(int grad);
    polynom();
    ~polynom(void);
    polynom& operator=(polynom p);
    friend ostream& operator<<(ostream& os, const polynom& p);
private:
    int grad;
    double* arr;
};

.cpp

polynom::polynom(int grad, double* arr)
{
    this->grad = grad;
    this->arr = arr;
}

polynom::polynom(int grad)
{
    this->grad = grad;
    this->arr = new double[grad];
}

polynom::polynom()
{
    arr = NULL;
}

polynom::~polynom()
{
    delete[] arr;
}

main

void main()
{
    double arr1[] = {5,0,1};
    double arr2[] = {3,2,1};
    polynom p1 = polynom(2, arr1);
    polynom p2 = polynom(2, arr2);
    system("pause");
}

Thanks a lot!

you are trying to delete stack allocated arrays, this would crash the app normally. you'd probably want to avoid using raw pointers and rather look at `td::shared_ptr` and `std::unique_ptr` — bobah, Nov 28 '13 at 20:55
That's apparently not the main issue, but don't forget to define a copy c`tor. — MaMazav, Nov 28 '13 at 20:56
possible duplicate of [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — n. m. could be an AI, Nov 28 '13 at 20:57

score 3 · Answer 1 · answered Nov 28 '13 at 20:56

You're running into The Rule of Three.

It will work if you do this:

polynom p1(2, arr1);
polynom p2(2, arr2);

The problem is your assignment operator does not duplicate the array, so when you assign the local temporary, it gets immediately destroyed (along with the array). Then when your locals go out of scope, they try to delete a pointer that has already been deleted.

Instead of overriding operator=, you should make a copy constructor polynom(const polynom&). You very rarely need to override operator=. The copy constructor is also more versatile - it will allow you to pass your object by value, whereas operator= is only specific to assignment.

In any case, instead of copying the pointer, you must allocate a new array and copy the contents. You should really use std::vector, and you wouldn't have these problems.

I'm sorry, I can't quite follow - I've changed my code to say `polynom p1(2,arr1);` etc., but it still won't work. Overriding the = operator is a restriction given by my professor. — Dennis Röttger, Nov 28 '13 at 21:00
Sorry, the other answer picked up the issue. I didn't check all of your constructors. Mine points out a different problem. — paddy, Nov 28 '13 at 21:02

Alexis Wilke · Accepted Answer · 2013-11-28T21:09:30.107

In the following you are assigning stack defined variables to the arr array in the polynom.

double arr1[] = {5,0,1};
double arr2[] = {3,2,1};
polynom p1 = polynom(2, arr1);
polynom p2 = polynom(2, arr2);

Once the polynom is destroyed, it deletes those pointers... You probably wouldn't write this:

double arr1[] = {1,2,3};
delete arr1;

Same problem.

--- Update

There are two possible solution to go around the problem:

the constructor that accepts user data allocates a buffer and makes a copy

This is the safest because that way the buffer inside the object is considered safe. In very large project, that will save you a lot of time in the long run, assuming you do not allocate millions of polynom objects... in which case slowness could become a problem.

the constructor marks the input as user input meaning that it does not have the right to delete it, the caller is responsible for that to happen

This works by adding a Boolean flag and if set to true (for example), the destructor does not delete the buffer.

There is, however, a huge problem with this method: if you use a stack based buffer and the polynom is returned, the buffer gets deleted by the return statement and you have a bug...

Therefore I do NOT recommend that you use this method ever.

--- Additional Note

You wrote polynom(2, arr1); ... note that you have THREE elements in the array. I would suggest you use sizeof() instead: polynom(sizeof(arr1)/sizeof(arr1[0]), arr1);.

Also, as mentioned by the other answer, you want to overload the polynom copy constructor and copy assignment because both will otherwise break your code. As long as you do not copy polynoms, though, you're fine. However, as it stands you would end up using the defaults and they won't work right at all.

Is there a way to assign stack defined variables and still be able to setup a "proper" destructor? - This way of entering the arrays was suggested by my professor. — Dennis Röttger, Nov 28 '13 at 21:03
You must allocate a new array and copy the contents of the array you were passed. Or use `std::vector`. — paddy, Nov 28 '13 at 21:04
Ah, thanks, I changed the 2 parameter constructor accordingly and now it works — Dennis Röttger, Nov 28 '13 at 21:05

c++ destructor de-allocation issue

2 Answers2