Hi

I get "debug assertion failure" (using Visual studio 2013) with:

Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

void world::deleteArmy(army * u){
	armyList.erase(u->curIt);	
	delete u;
}

It happens on the delete row. "u" is still pointing to a valid instance of the army class. The armyList is a std-list of type <* army>.

What can cause this? Is the iterator messed up maybe so when I erase the iterator the list gets corrupted? But shouldnt it complain on that row and not on the delete row?

Thanks a lot!
Erik

When you erase one iterator, it is likely to invalidate some or all other iterators into that container, depending on what the container type is.

This means that storing iterators inside your army object is a bad idea if you intend erasing them at various points, as they can become invalid.

That in turn might mean that wherever you are getting these army pointers from is pulling them from invalid places - but that part is impossible to see without looking at more code.

If i switch from 

std::list<army *>
to
std::list<army>

Then I wouldnt need iterators at all right? (when removing). Storing pointers is an old idea I got, I'm not sure it makes any idea for my game, better to just store the objects directly in the std::list right?

You don't need iterators to remove anyway, since you could remove based on the pointer value. I wouldn't recommend storing full objects in a vector because you'll probably run into other issues when you copy them around. It might make more sense to store shared_ptr<Army>.

Well I need iterators if i store them loosely in memory and only stores pointer in the list, right? How would I otherwise know which pointer to remove from the list when i delete an army? I could do it with unique ids maybe.

But what i need is basically:

Loop through all the armies
Remove (anywhere, preferably be able to do it inside the loop)
Add (insert in beginning or end is fine)

I already have made functions for load/save to disk (if they are in a std::list) which is nice. But that would work even if i go from <army*> to <army>. There is some error in the code where I restore them from disk i think.

But basically it would make sense to skip using pointers in the list and simply use the objects themselves in the list, right?

You don't necessarily need iterators. As you say, you can do it by IDs. Or you can do it by pointer, because if you were storing pointers in the container, they don't change even when the container grows and shrinks. The pointer way is potentially dangerous if you aren't sure whether the object you point to is still in existence when you try to delete it. But so are iterators, if you don't know whether they got invalidated or not. With an std::list, they almost never get invalidated, unless you erase the very element that an iterator currently points to - which is probably exactly what your current design does.

If you do need to remove an object from a std::list while iterating over it, the answer is actually quite simple - make sure you've incremented the relevant iterator before you erase it. (Usually this means copying the iterator, incrementing it, then erasing whatever the copy points to.) This is obviously not practical when each object is holding its own iterators as you'd have to go in to each object and see if they have a copy of that iterator and clear it.

You can always store 'full' objects instead of std::list instead of pointers to them. In that case, you have to be careful if you ever move to using std::vector, because the semantics are different. I mentioned storing pointers above because I'd misread and thought you were using vectors already (most people don't use std::list), but with a list storing the full object is more reasonable. This is largely independent of what you do with iterators, however.

Cool, thanks for the help!

Im still confused. I dropped the use of iterators inside the army itself, like so:

void world::deleteArmy(army * u){

	std::list<army *>::iterator it;
	for ( it = armyList.begin(); it != armyList.end(); ++it ){
		army * uu=*it;
		if (uu->id == u->id){
			armyList.erase(it);
			break;
		}
	}
	delete u;
}

This still gives me debug assertion failure on the "delete u;" row. Im not using the stored iterators anymore, what could be the problem?

It happens only when trying to delete an army that i restored from a savefile, but i though the error lied in corrupted iterators. Or could that still cause the problem even if i dont use those for deletion? (i use the iterator from looping though the list as seen above).

How do we know that 'u' is valid when you call this function?

Comment out that loop - do you still see the problem?

And are you confident that you only store and reference army objects in one place?

Ah sorry! The problem remains even if I comment out everything except "delete u;"

The pointer seems to be valid though. I can check its members and those are intact. Can it be that it checks the iterator inside of army when I delete it? And it complains that it's somehow corrupted?

And can I then "uncorrupt" the iterator? In the long run i will remove it entierly from the army class, but i would like to solve this first.