CODEWITHSUNDEEP
c++memoryvector
3Pi
3Pi

Reputation: 1914

C++ Memory Error on Delete - Debug Assertion Failed

I am trying to add a set of new ModelImages to a vector, and am getting an error, Debug Assertion Failed, Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse). This occurs when trying to delete the second ModelImage that is generated.

std::vector<ModelImage> ModelImages;

for(int n=0;n<nParamSets;n++)
{
    ModelImage* mI = new ModelImage(MOD_WIDTH,MOD_HEIGHT);
    ModelImages.push_back(*mI);
    delete mI;
}

The constructor and destructor, and copy and swap funcitons, are as follows:

ModelImage(int _width, int _height)
{
    width = _width;
    height = _height;
    nPixels = width*height;
    distance =  new float[nPixels];
    intensity = new float[nPixels];
    derivX =    new float[nPixels];
    derivY =    new float[nPixels];
    maxDistance = 0.0f;
    minDistance = 0.0f;
}

~ModelImage()
{
    delete [] derivX;
    delete [] derivY;
    delete [] distance;
    delete [] intensity;
}

ModelImage& operator=(ModelImage other)
{
    swap(*this, other);
    return *this;
} 

friend void swap(ModelImage& first, ModelImage& second)
{
    using std::swap;
    swap(first.derivX,second.derivX);
    swap(first.derivY,second.derivY);
    swap(first.distance,second.distance);
    swap(first.intensity,second.intensity);

    swap(first.nPixels,second.nPixels);
    swap(first.width,second.width);
    swap(first.height,second.height);
}

Just before trying to delete the second ModelImage, looking at the vector ModelImages shows that the two ModelImages in the vector have the same assigned memory addresses for the distance, intensity, derivX, derivY arrays.

Any help is appreciated, thanks.

Upvotes: 1

Views: 1304

Answers (4)

moooeeeep
moooeeeep

Reputation: 32512

My first guess is that you don't have a copy constructor defined. The vectors' push_back will default copy construct your ModelImage which will simply copy the member pointers but not reallocate the memory they point to. However, these references will be gone after the original objects are deleted.

Hint: A copy constructor is something like:

ModelImage(const ModelImage& orig) {
  // appropriately reinitialize from orig
}

Not to confuse with the assignment operator==

Why do you do create these ModelImages dynamically anyway (if you throw them right away)? And why don't you take vector<float>(nPixels) instead of new float[nPixels]?

Upvotes: 1

rerun
rerun

Reputation: 25505

I'm assuming that you have all of the arrays defined as pointers in the class. The default copy copies the values of the pointer meaning that when you delete the pointer in the outer function you delete the underlining memory.

Just a couple of suggestions

-Utilize Vector instead of a float * std::vector has copy and move constructors defined allread

-The loop doesn't need to use the free store at all value semantics and coping are fully supported and less error prone. 

 for(int n=0;n<nParamSets;n++)
 {       
     ModelImages.push_back( ModelImage(MOD_WIDTH,MOD_HEIGHT));
 }

Upvotes: 0

RC.
RC.

Reputation: 28207

This is likely due to you not having a copy constructor.

Create a copy constructor that makes copies of the memory that your pointers are referencing.

When using std containers, they usually will create copies of your object as you insert. As you have no copy constructor, all your member pointers end up pointing to the same memory address because it's simply doing a member-wise copy of your data. Once one of the temporary copies is destructed, (or when you call delete on the original object after the insert) the memory of the object inserted has had it's memory deleted from under it.

Upvotes: 3

Michael Burr
Michael Burr

Reputation: 340208

It's not clear from what you've posted whether or not you have a proper copy constructor and assignment operator for the following members:

distance
intensity
derivX
derivY

If not, you need those. (see Rule of three (C++ programming) for a bit more information).

A better alternative would be to use std::vector<double> for those data members. That way copying, assignment, and destruction would all be handled automatically. You'd still want to construct them to have the proper number of elements.

Upvotes: 0

Related Questions