Pointer being freed was not allocated – destructor

Slowly learning about copy/move constructors, rule of 5 etc. Mixed with not-so-well understanding of usage of pointers/reference, I cannot understand why my destructor throws the error below. I know that it’s caused by destructor and destructor only, and after the copy constructor.

untitled(19615,0x104a60580) malloc: *** error for object 0x600000b74000: pointer being freed was not allocated

untitled(19615,0x104a60580) malloc: *** set a breakpoint in malloc_error_break to debug

Code:

#include <string>

using namespace std;
typedef int size_type;

class user{
public:
    user():
    rank(1),
    name("default"){
    }
private:
    int rank;
    string name;
};

class account{
public:
    account(size_type max_size):
    owners(new user[max_size]),
    max_size(max_size){
        cout<<"normal constructor"<<endl;
    }
    account(account& other):
    owners(other.owners),
    max_size(sizeof(owners)){
        cout<<"copy constructor called"<<endl;
    }
    account(account&& other):
    owners(other.owners),
    max_size(sizeof(owners)){
        cout<<"move constructor called"<<endl;
    }
    ~account(){
        if(owners!= NULL) {
            delete[] owners;
            owners= 0;
        }
        cout<<"destructor called"<<endl;
    }

private:
    user* owners;
    size_type max_size;
};

int main(){
    account f(3);
    account n(f);
}

>Solution :

I cannot understand why my destructor throws the error below. I know that it’s caused by destructor and destructor only

account f(3);

The constructor of this object allocates an array.

account n(f);

The copy constructor that you wrote copies the pointer that points to the dynamic array.

n is destroyed first. Its destructor deletes the array. f is destroyed after that. Its destructor also deletes the same array and the behaviour of the program is undefined, which lead to the output that you observed.

To prevent the allocation from being deleted multiple times, you must enforce a class invariant (a post condition that applies to all member functions) that any non-null owners value must be unique across all instances of the class. To maintain that invariant in the copy constructor (as well as move constructor and copy and move assignment operators), you must not copy the pointer, as you do now. I recommend studying the RAII idiom.

Better solution: Don’t use owning bare pointers. Use std::vector<user> and don’t have user defined special member functions at all.

P.S. The copy constructor should accept a reference to const.

Also:

if(owners!= NULL) {
    delete[] owners;
    owners= 0;
}

The check is redundant because it’s safe to delete null. The assignment is redundant because the object is about to be destroyed and the member cannot be accessed after that anyway.