C++ destructor issue with std::vector of class obj

Question

I am confused about how to use destructors when I have a std::vector of my class.

So if I create a simple class as follows:

class Test
{
private:
 int *big;

public:
 Test ()
 {
  big = new int[10000];
 }

    ~Test ()
 {
  delete [] big;
 }
};

Then in my main function I do the following:

Test tObj = Test();
vector<Test> tVec;
tVec.push_back(tObj);

I get a runtime crash in the destructor of Test when I go out of scope. Why is this and how can I safely free my memory?

Answer 1

The problem is you don't define a copy constructor for Test. So the compiler generates a default copy constructor for you, which just copies the content of the object - in this case the int pointer.

Now, when you push back your object into the vector, it is implicitly copied with the copy constructor. Which results in two objects pointing to the same array of ints! So in the end, two destructors try to delete the same array - BANG.

Whenever you define a class which owns members via pointers*, apart from the destructor you must also define a copy constructor for it. Update: and an assignment operator, for the same reason (thanks @James :-)

Update2: A trivial way to get around all these restrictions is to define a static array instead of the dynamically allocated one:

class Test
{
private:
  int big[10000];
  // no need for constructors, destructor or assignment operator
};

However, the best practice is to use std::vector<int> instead of an array.

* that is, contains pointers to members with ownership semantics (thanks to @Steve Jessop for clarification)

Answer 2

Without a copy-constructor, the vector will create a flat copy of your object. This leads to two objects of type Test referencing the same array big. The first instance deletes the array when it gets destroyed, and then the second instance try to dereference a deleted pointer, which is undefined behavior.

Answer 3

Your problem is here:

Test tObj = Test();

The Test() creates a temporary Test object, which then gets copied to tObj. At this point, both tObj and the temporary object have big set to point to the array. Then the temporary object gets destroyed, which calls the destructor and destroys the array. So when tObj gets destroyed, it tries to destroy the already-destroyed array again.

Further, when tVec is destroyed, it will destroy its elements, so the already-destroyed array will be destroyed yet again.

You should define a copy constructor and an assignment operator so that when a Test object gets copied, the big array gets copied, or has some sort of reference count so that it doesn't get destroyed until all owners are destroyed.

An easy fix is to define your class like this:

class Test
{
private:
 std::vector<int> big;

public:
 Test (): big(10000) {}
};

In this case, you wouldn't need to define any destructor, copy constructor, or assignment operator, because the std::vector<> member will take care of everything. (But note that this means 10,000 bytes get allocated and copied whenever you copy an instance of Test.)

Answer 4

Test tObj = Test();

This is wrong and should be as it does not create copies:

Test tObj;

This also create a lot of copies:

 vector<Test> tVec;
 tVec.push_back(tObj);

So if you free one int array, you'll free all the arrays. And the following delete will fail.

What you need is either:

• use a copy constructor to have for each class a separate array

• Why use a pointer?

class Test    
{   
private:  
  int big[10000];    
public:

};

This will work fine.