Double free inside destructor when added to vector
Hey, I'm working on a drum machine and I'm having problems with vectors.
Each sequence has a list of samples, and the samples are ordered in a vector. However, when the sample is push_back on a vector, the fetch destructor is called and results in a double error.
Here is the code for creating the sample:
class XSample
{
public:
Uint8 Repeat;
Uint8 PlayCount;
Uint16 Beats;
Uint16 *Beat;
Uint16 BeatsPerMinute;
XSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat);
~XSample();
void GenerateSample();
void PlaySample();
};
XSample::XSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat)
{
Beats = NewBeats;
BeatsPerMinute = NewBPM;
Repeat = NewRepeat-1;
PlayCount = 0;
printf("XSample Construction\n");
Beat = new Uint16[Beats];
}
XSample::~XSample()
{
printf("XSample Destruction\n");
delete [] Beat;
}
And the Dynamo code that creates each sample in the vector:
class XDynamo
{
public:
std::vector<XSample> Samples;
void CreateSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat);
};
void XDynamo::CreateSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat)
{
Samples.push_back(XSample(NewBeats,NewBPM,NewRepeat));
}
Here's main ():
int main()
{
XDynamo Dynamo;
Dynamo.CreateSample(4,120,2);
Dynamo.CreateSample(8,240,1);
return 0;
}
And this is what happens when the program starts:
Starting program: /home/shawn/dynamo2/dynamo
[Thread debugging using libthread_db enabled]
XSample Construction
XSample Destruction
XSample Construction
XSample Destruction
*** glibc detected *** /home/shawn/dynamo2/dynamo: double free or corruption (fasttop): 0x0804d008 ***
However, when the delete [] is removed from the destructor, the program works fine.
What is causing this? Any help is appreciated.
a source to share
The problem is that you are dynamically allocating memory to your object, but you are not declaring a copy / assignment constructor statement. When you allocate memory and are responsible for removing it, you need to define all four methods that the compiler generates.
class XSample
{
public:
// Pointer inside a class.
// This is dangerous and usually wrong.
Uint16 *Beat;
};
XSample::XSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat)
{
// You allocated it here.
// But what happens when you copy this object or assign it to another variable.
Beat = new Uint16[NewBeats];
}
XSample::~XSample()
{
// Delete here. Turns into double delete if you don't have
// copy constructor or assignment operator.
delete [] Beat;
}
What happens with the above:
XSample a(15,2,2);
XSample b(a); // Copy constructor called.
XSample c(15,2,2);
c = a; // Assignment operator called.
Two ways to solve this problem:
- Create a copy / assignment constructor.
- Use a different object that manages memory.
I would use solution 2 (as it is easier).
Its also the best design. Memory management should be done by their own class and you should concentrate on your drums.
class XSample
{
public:
std::vector<Uint16> Beat;
};
XSample::XSample(Uint16 NewBeats,Uint16 NewBPM,Uint8 NewRepeat):
Beat(NewBeats)
{
// Notice the vector is constructed above in the initializer list.
}
// Don't need this now.
XSample::~XSample()
{
}
If you want to do it the hard way:
Dynamically allocate an array of objects
If you want to see what the compiler versions look like:
C ++ implicit copy constructor for a class that contains other objects
a source to share
You need a correct constructor and assignment operator, since you have a non-trivial destructor (more precisely, because your class preserves memory allocation). See the Big Rule 3:
Update:
As Martin York mentions in the comments, this answer really just fixes the immediate cause of the problem, but doesn't really suggest the best way to fix it, which is to use RAII members that automatically manage resources. On the front (subject to the example code), the member Beat
can be std::vector<>
instead of a pointer to a hand-selected array. The element vector<>
allows the class to not need a special dtor, copy ctor, or assignment - all of these parts will be automatically provided for the member Beat
, if any vector<>
.
a source to share
What happens is what Samples.push_back()
copies its argument to the vector. Since it XSample
does not have a specific copy constructor, the compiler generates a default constructor that performs a shallow copy. This means that the pointer in Beat
both the original and the copy in the vector points to the same memory. The original is then destroyed at the end push_back()
, deleting the pointer Beat
.
At the end, main is Dynamo
destroyed by calling the destructor of each of the elements. This is trying to delete an already deleted pointer Beat
, which will result in a double delete error.
a source to share