Double free or corruption after queue::push

Question

#include <queue>
using namespace std;

class Test{
    int *myArray;

        public:
    Test(){
        myArray = new int[10];
    }

    ~Test(){
        delete[] myArray;
    }

};


int main(){
    queue<Test> q
    Test t;
    q.push(t);
}

After I run this, I get a runtime error "double free or corruption". If I get rid of the destructor content (the delete) it works fine. What's wrong?

Answer 1

Let's talk about copying objects in C++.

Test t;, calls the default constructor, which allocates a new array of integers. This is fine, and your expected behavior.

Trouble comes when you push t into your queue using q.push(t). If you're familiar with Java, C#, or almost any other object-oriented language, you might expect the object you created earler to be added to the queue, but C++ doesn't work that way.

When we take a look at std::queue::push method, we see that the element that gets added to the queue is "initialized to a copy of x." It's actually a brand new object that uses the copy constructor to duplicate every member of your original Test object to make a new Test.

Your C++ compiler generates a copy constructor for you by default! That's pretty handy, but causes problems with pointer members. In your example, remember that int *myArray is just a memory address; when the value of myArray is copied from the old object to the new one, you'll now have two objects pointing to the same array in memory. This isn't intrinsically bad, but the destructor will then try to delete the same array twice, hence the "double free or corruption" runtime error.

How do I fix it?

The first step is to implement a copy constructor, which can safely copy the data from one object to another. For simplicity, it could look something like this:

Test(const Test& other){
    myArray = new int[10];
    memcpy( myArray, other.myArray, 10 );
}

Now when you're copying Test objects, a new array will be allocated for the new object, and the values of the array will be copied as well.

We're not completely out trouble yet, though. There's another method that the compiler generates for you that could lead to similar problems - assignment. The difference is that with assignment, we already have an existing object whose memory needs to be managed appropriately. Here's a basic assignment operator implementation:

Test& operator= (const Test& other){
    if (this != &other) {
        memcpy( myArray, other.myArray, 10 );
    }
    return *this;
}

The important part here is that we're copying the data from the other array into this object's array, keeping each object's memory separate. We also have a check for self-assignment; otherwise, we'd be copying from ourselves to ourselves, which may throw an error (not sure what it's supposed to do). If we were deleting and allocating more memory, the self-assignment check prevents us from deleting memory from which we need to copy.