Dynamic array deletion

Question

I'm working with dynamic arrays and there is the problem I've got. Compiler calls destructor for the first array (with length 1) twice. So the program crashes. I'd be glad for any help. Thank you.

class MyClass{
public:
    int *a;
    MyClass(int i){
        a = new int[i];
    }
    ~MyClass(){
        if (a) delete[]a;
    }
};


int main(){ 
    MyClass c(1);
    c = MyClass(2);
}

Answer 1

The program crashes because copying MyClass only copies the pointer member and does not create a new array which then can be deleted. So you delete the same array twice which is Undefined Behavior.

The best fix for that is using std::vector<int> instead of raw arrays if every instance should have its own array.

If you (for some artificial restriction, there is no actual usecase for that) need to use raw arrays, define a suitable copy constructor and assignment operator. See "The rule of Three".

Answer 2

Why you copy a MyClass instance, the default-generator copy constructor is called. This just copies the internal a pointer, and does not create a new int array.

So when the copy is destroyed, it deletes the array, and then when the original is destroyed, it also deletes the array.

There are 2 ways around this:

1. Preferable: use something that wraps up all the allocation and copying for you, such as std::vector (rule of 0, we'll come to that in a minute), or if you are trying to create such a class, say for educational interest,
2. Follow the "rule of 5" (used to be 3, before C++11 introduced move semantics). Whenever you have a data member that needs some special creation/destruction, make sure you deal with it appropriately in all constructors and destructors: copy/move construction/assignment, and destruction.

Note that at the moment, it is impossible to properly write a copy constructor/assignment operator, since you don't store the size of the array, so you don't know what size the copy should be.

For instance, if you store the array size in an int member named i, your copy constructor might look like:

MyClass(const MyClass& rhs) : a(0), i(rhs.i) {
    a = new int[i];
    std::copy(rhs.a, rhs.a+rhs.i, a);
}

Writing a copy assignment operator is a bit trickier, since you need to deal with deleteing the existing array, and even with restoring the original state if allocating the new array throws an exception:

Edit: sorry, my previous operator= example was total crap, this version is better:

MyClass& operator=(const MyClass& rhs) {
    int *newA = new int[rhs.i];
    std::copy(rhs.a, rhs.a+rhs.i, newA);
    i = rhs.i;

    delete[] a;
    a = newA;

    return *this;
}

You only replace the existing member after building the new one. This way, if allocating the new one throws an exception, the object is still in its old state.

Answer 3

This occurs because the same "a" was deleted twice. What happened:

1. MyClass c(1) // allocated a at address 1
2. MyClass(2) // allocated a at address 2
3. c = ... // now c.a points to address 2, because c just copies every members of the object on the right side
4. object created at 2. was destroyed, thus address 2 is freed
5. c was destroyed, thus address 2 is freed for the second time, causing crash

The solution is not to copy the address of "a", but create a copy constructor and copy the value of "a" instead.