constructor not being called and "pointer being freed was not allocated" error

Question

I've started picking up C++. The concepts are hard since I don't have any prior object-oriented programming experience. I am learning built-in arrays and constructors/destructors, and I have encountered an error that I cannot wrap my head around. Here, I'll show you guys the code.

The class definition goes

class arraysClass{

public:

    int size1;
    int size2;

    int **frequencies;

    arraysClass(int size1, int size2){
        cout << "Constructor: size1&size2 " << size1 << " " << size2 << endl;
        frequencies = new int*[size1+1];
        for (int i=0; i<=size1; i++){
            frequencies[i] = new int[size2+1];
        }
    }

    //Destructor
    ~arraysClass()
    {
        cout << "Destructor: size1&size2 " << size1 << " " << size2 << endl;
        for (int i=0; i<=size1; i++){
            delete [] frequencies[i];
        }
        delete [] frequencies;

    }
};

and here is my main function

int main()
{
    int size1 = 20;
    int size2 = 30;
    arraysClass arrays1(size1, size2);

    arraysClass arrays2 = arrays1;

    arrays2.size1 = size1;
    arrays2.size2 = size2;

    return 1;
}

What I get as the result is

Constructor: size1&size2 20 30

Destructor: size1&size2 20 30

Destructor: size1&size2 0 0

a.out(41138,0x7fff75694000) malloc: *** error for object 0x7fa2eac032d0: pointer being freed was not allocated

The weird thing is that the constructor was only called once, when there is clearly a second instance of my class object. I have not made any copy constructors or overloaded the operator= function, so I'm not really sure what to make of this situation.

Anyone care to help a hopeless college student? Thanks!

The implicit copy constructor was called, and the 2nd instance tries to delete the copied pointer again. — πάντα ῥεῖ
Also your new int*[size1+1] with for... i <= size1 is not ideomatic C++ and makes your code less consistent and harder to follow; and indeed, because you rely on size1 in your destructor you risk a crash if your change size1. You should make all of your class members private and the size values immutable. — Dai
@πάνταῥεῖ Thanks for your lead. I did some googling on the subject but still quite fuzzy on how copy constructors work. So does this mean that both arrays1 and arrays2 point to the same object? Is this what implicit copy constructors are supposed to do for user-defined classes? I guess it would explain why I get such an error message. — Byng
C++ has an informal rule, called the Rule of Three: if you need any of the three special members { copy constructor, assignment, destructor }, then you probably need all three. You may also want to implement a move, but that's a performance optimization. — MSalters

Starl1ght Starl1ght · Accepted Answer · 2016-11-29T06:21:43

arraysClass arrays2 = arrays1; Here default copy constructor was invoked and member int **frequencies was copied to arrays2. The catch is - this is so called, shallow copy, i.e. only pointer value is copied and both pointers pointing to same memory.

At the end of main function, both arrays1 and arrays2 are destroyed, and in destructor, int **frequencies (remember, they are pointing to same location) deleted twice. And so, first delete is going fine, second one is crashing program, because deleting already deleted pointer is undefined behavior.

Correct fix here is to make deep copy in copy constructor and overloaded operator=. Example with copy constructor:

arraysClass(const arraysClass& rhs) {
    cout << "Copy Constructor" << endl;
    size1 = rhs.size1;
    size2 = rhs.size2;
    frequencies = new int*[size1+1];
    for (int i=0; i<=size1; i++){
        frequencies[i] = new int[size2 + 1];
        memcpy(frequencies[i], rhs.frequencies[i], rhs.size2 + 1)
    }
}

constructor not being called and "pointer being freed was not allocated" error

1 Answers