Destructor of an object causes crashing

Question

When I run the below code, it crashes. If I remove the destructor, it works. Why?

#include <iostream>

#include <stdlib.h>

#include <stdarg.h>

using namespace std;

class vectmy {
    public: int size;
    int * a;
    vectmy(int n, int val);
    ~vectmy() {
        delete[] a; //IF I DELETE THIS PROGRAM WORKS
    }
    vectmy & operator = (const vectmy & );
};

vectmy::vectmy(int n, int val) {
    size = n;
    a = new int[n + 1];
    for (int i = 0; i < n; ++i) {
        *(a + i) = val;
    }
}

vectmy & vectmy::operator = (const vectmy & param) {
    for (int i = 0; i < 3; ++i) a[i] = param.a[i];
    return * this;
}

vectmy operator + (vectmy left, vectmy right) {
    vectmy result = left;
    for (int i = 0; i < 3; ++i) result.a[i] = result.a[i] + right.a[i];

    return result;
}

int main() {
    int b1[3] = {
        1,
        2,
        4
    };
    vectmy d(3, 2), b(3, 4), c(3, 0);

    c = (b + d);

    for (int j = 0; j < 3; ++j) cout << c.a[j] << ' ' << endl;
    return 0;
}

Answer 1

When I run it crashes. If I remove the destructor it works. Why it happens?

Your operator + creates a copy of left here:

vectmy result = left;

Since you haven't defined a copy constructor explicitly, the compiler will generate one implicitly that performs member-wise copying.

A dull copy of the a data member means that the a pointer will eventually point to the same location for two different instances of vectmy (result and left), both of which will delete[] it upon destruction.

Such double deletion gives your program undefined behavior, which in your case manifests itself as a crash.

This is the point of the Rule of Three: every time you have a user-defined copy-constructor, assignment operator, or destructor, you should probably define all of them.

The reason is that you normally define one of those functions because you are managing some resource (memory, in your case), and you usually want to perform proper actions when copying, destructing, or assigning objects that manage a resource.

In this particular case, a proper copy constructor is missing. This is how you could define it:

vectmy::vectmy(vectmy const& v)
{
    size=v.size;
    a = new int[size];
    *this = v;
}

Also, I would suggest you to avoid manual memory management through raw pointers, new, and delete (or their array counterparts) whenever you can, and consider using std::vector instead.

UPDATE:

Also notice, that your operator + is accepting its parameters by value, which means that each argument will be copied (i.e., the copy constructor will be invoked).

Since a copy is not really necessary here, you may want to take your parameters by reference (to const):

vectmy operator + ( vectmy const& left, vectmy const& right)
//                         ^^^^^^              ^^^^^^