*** glibc detected *** double free or corruption (C++)

graemef · 01-06-2006, 12:14 PM

The top two are because the objects are passed by value into add. switch the signature to

Code:

void add(foo& f) { v.push_back(f); }

and they will go away

graeme

xhi · 01-06-2006, 12:31 PM

gotcha!

so in the end.. it may be a delete that is going bad that is not being called explicitly..

like you said graeme in the beginning it would be best to be passing by ref, for efficiency if nothing else..
but surely this should still work if you wanted to pass everything by value... ?

graemef · 01-06-2006, 01:22 PM

I think so although I'd be in favour of pointers but you do need to be careful not to delete then whilst they are still on the vector, as in the implicit delete in the bar class of your example. But the use of new should overcome that.

As for the original problem I suspect it is an implicit delete but I'm not totally certain. Anyway it was fun!

graeme.

xhi · 01-06-2006, 02:17 PM

>> Anyway it was fun!

yes sir!

and urzumph,
check out how graemef changed the vector to a vector of ptrs ..

vector<foo*> v;

and how the change affected all the other functions to handle refrences and read from pts out of the vector.. this is truly the correct way to do this type of thing.. way more efficient.. regardless of why the way you were doing it wont work..
though it is a mystery still to me.. as why..

anyhow though.. Good luck with it!

urzumph · 01-06-2006, 06:43 PM

Firstly, thanks everyone for your replies.

Quote:

Originally Posted by xhi

this is truly the correct way to do this type of thing.. way more efficient.. regardless of why the way you were doing it wont work.

While I may well change additem() to take a pointer, the vectors have to store the actual value because the copy outside of the list is overwritten shortly after the call to additem()
(although that probably could be changed)

Quote:

Originally Posted by xhi

i would consider compiling it if you wanted to upload the code..

I have uploaded it now, you can find it at
It's got all the source files as well as my Makefile (which doesn't work that well, but that's another story)
http://urzumph.homelinux.org/CalanderThingy.zip

Now, on to the interesting bit.
I noticed when apt-get updateing that there were new versions of some of the things I was using - gcc, glibc, libstdc++ etc, so I updated and tried again. Now, I am getting the same error (except different number), but in a different place. The error seemed to be coming from the resize function...

Code:

Checking to see if calander toadd is the same calander 0
Checking to see if calander toadd is the same calander 1
toadd passed required checks. adding Calander...
pushed back score
pushed back calander
Reordering BestOfList...
Resizing BestOfList...
*** glibc detected *** double free or corruption (!prev): 0x08054558 ***
Added Item to new listmake: *** [test] Aborted

(plese note that in the old logs, they were Reordering calander and resizing calander, I realised these were misleading and fixed them)

Now, I had some vector.erase() calls in resize, and I'm not sure I got them right, so for the meantime, I simply commented them out, leaving this function :

Code:

void BestOfList::resize() {
	int lastscore = Scores[Scores.size()-1];
	printf("Resizing BestOfList...\n");
	if(Scores.size() < minsize) {return;}
	for(size_t i = Scores.size()-1;i>=minsize;i--) {
		if(Scores.size() < maxsize) {return;}
		if(lastscore < Scores[i]) {
//Scores.erase(Scores.begin()+i+1,Scores.end());
//Calanders.erase(Calanders.begin()+i+1,Calanders.end());
		}
	}
}

(In theory, I needn't have commented them out because Scores.size() should always be less than minsize with this data)

Apparently, this did not stop the problem however.
I am begining to wonder if perhaps stdout needs to be fflush()'d

and lo and behold :

Code:

Adding to list will continue - score >= 0
Checking to see if calander toadd is the same calander 0
Checking to see if calander toadd is the same calander 1
toadd passed required checks. adding Calander...
pushed back score
pushed back calander
Reordering BestOfList...
Resizing BestOfList...
Added Item to new list
*** glibc detected *** double free or corruption (!prev): 0x08054558 ***

So I'll go and add fflush() calls all over the place and try and track down where this error is

(Sorry if this post seems to ramble a bit, I descovered some of the stuff while writing it)

Edit - Tried to fix indentation of code

xhi · 01-06-2006, 06:47 PM

excellent catch! i just downloaded the source and will take a look at it later...

urzumph · 01-06-2006, 11:33 PM

Ok, I managed to isolate it down to a single line again, this time in the main function :

Code:

oldlist = newlist;

where oldlist and newlist are both BestOfLists.
Now, I figgured that from a C++ perspective, the way I had done this was probably quite silly...

Code:

(snip)
BestOfList oldlist;
BestOfList newlist;
(snip)
while(!iscomplete(oldlist.getcalander(0))) {
    for(unsigned int i = 0; i < oldlist.length(); i++) {
        (snip data processing)
    }
    oldlist = newlist;
    newlist.empty();
}

now I realised in C++ with pointers this was silly, because I could simply swap pointers around:

Code:

(snip)
BestOfList *oldlist, *newlist;
oldlist = new BestOfList;
newlist = new BestOfList;
(snip)
while(!iscomplete(oldlist->getcalander(0))) {
    for(unsigned int i = 0; i < oldlist->length(); i++) {
        (snip data processing)
    }
    printnow("Replacing Oldlist with Newlist\n");
    delete oldlist;
    printnow("Oldlist deleted");
    oldlist = newlist;
    newlist = new BestOfList;
}

And the Line it doesn't like is delete oldlist;.
You don't need to do anything special when you delete instances of classes with vectors in them do you?

xhi · 01-06-2006, 11:56 PM

>> You don't need to do anything special when you delete instances of classes with vectors in them do you?

no you dont.. well if it is vector of dynamically allocated objects you ofcourse have to delete themm
if you did..

vector<foo*> v;
v.push_back(new foo);

then you would have to delete it from vector.. (still shouldnt give this error..)

otherwise the vector can just die when the object is destructed...

btw i compiled and ran the source you sent and im getting a segfault.. maybe somthing out of range now?

here is the output.. im trying to get another project done for school or i would try to help a little..

Code:

(snipped)
Adding appointment 1
Calander already has appointment in this position. Failing calander.
Scoring Calander....
Fail condition is true. Returning -1
Added Item to new listGetting Calander 1 from BestOfList
Appointment at 0,11
Adding appointment 2
Calander already has appointment in this position. Failing calander.
Scoring Calander....
Fail condition is true. Returning -1
Added Item to new listGetting Calander 1 from BestOfList
Appointment at 0,14
Adding appointment 3
Testing Cat 0 against our Cat
Testing Cat 1 against our Cat
Scoring Calander....
Fail condition is true. Returning -1
Added Item to new listGetting Calander 0 from BestOfList
Segmentation fault
!?

graemef · 01-07-2006, 08:00 AM

Just to add my view on the best way to use pointers in this vector.

If the vector is to be your primary storage of the object then the approach should be as follows:

Have a pointer to the object
Create the object using the new keyword
Pass the pointer to the method that pushes it onto the vector
Reuse the pointer to create another object (again using new)

When you delete the vector you will need to go through each element and free the object using the delete keyword. This can be done in the destructor of the object that holds the vector. This gives you a consistent way of managing the data that you allocate as well as making the vector itself efficient.

graeme.

dmail · 01-07-2006, 09:03 AM

Quote:

When you delete the vector you will need to go through each element and free the object using the delete keyword.

just a word of caution about this.
say for example you have a vector of int pointers;
std::vector<int*> ints;

you can not do this

Code:

//delete all the pointers
std::vector<int*> ::iterator iter;
//loop through the vector
for(iter = ints.begin(); iter != ints.end(); ++iter)
{
     delete (*iter);//recover the memory
     ints.erase(iter);//remove from vector
}

the reason for this is that you are removing the iter and then incrementing an iter which no longer is valid.

theres a couple of ways of actually doing this correct, heres two

Code:

//delete all the pointers
std::vector<int*> ::iterator iter;
//loop through the vector
for(iter = ints.begin(); iter != ints.end(); ++iter)
     {delete (*iter);}//recover the memory

ints.erase(ints.begin(),ints.end());//clean the vector

Code:

while( ! ints.empty() )
{
     delete ints.back();//recover the memory
     ints.pop_back();//remove from vector
}

urzumph · 01-07-2006, 04:03 PM

Quote:

Originally Posted by xhi

btw i compiled and ran the source you sent and im getting a segfault.. maybe somthing out of range now?

That is really wierd. I downloaded and tested the source myself and I got the double-free. If all 3 calanders fail, as per your log (and they should not! assuming you were using the default data) Then yes, it is likely there will be an out of bounds error in one of the vectors, probably causing the segfault.