Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Double free or corruption after queue::push

#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?

like image 318
Mihai Neacsu Avatar asked Dec 28 '12 02:12

Mihai Neacsu


People also ask

What causes double free or corruption C++?

The error of double free or corruption in C++ means that our program somehow invokes the free() C++ object with an illegal pointer variable. When we use smart pointers such as shared_ptr, we must check because if we call the function get(), we are directly using the raw pointer.

What is double free or corruption?

A double free or corruption likely means that free was called twice on the same block of memory, or that something was overwritten that shouldn't have been, e.g. an array overrun or something similar. This might have happened deep within Julia itself or in some C library that your code calls.

How to avoid double free in c++?

Double Free A simple technique to avoid this type of vulnerability is to always assign NULL to a pointer after it has been freed. Subsequent attempts to free a null pointer will be ignored by most heap managers.


2 Answers

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.

like image 64
derekerdmann Avatar answered Oct 01 '22 13:10

derekerdmann


The problem is that your class contains a managed RAW pointer but does not implement the rule of three (five in C++11). As a result you are getting (expectedly) a double delete because of copying.

If you are learning you should learn how to implement the rule of three (five). But that is not the correct solution to this problem. You should be using standard container objects rather than try to manage your own internal container. The exact container will depend on what you are trying to do but std::vector is a good default (and you can change afterwords if it is not opimal).

#include <queue> #include <vector>  class Test{     std::vector<int> myArray;      public:     Test(): myArray(10){     }     };  int main(){     queue<Test> q     Test t;     q.push(t); } 

The reason you should use a standard container is the separation of concerns. Your class should be concerned with either business logic or resource management (not both). Assuming Test is some class you are using to maintain some state about your program then it is business logic and it should not be doing resource management. If on the other hand Test is supposed to manage an array then you probably need to learn more about what is available inside the standard library.

like image 43
Martin York Avatar answered Oct 01 '22 13:10

Martin York