World::PurgeDebugDrawing() uses invalid iterator

[GoogleCodeExporter]

    DebugDrawIterator itdd = _debugDrawItems.begin();
    while (itdd != _debugDrawItems.end())
    {
        DebugDrawBase* pDD = (*itdd);
        _debugDrawItems.erase(itdd);
        delete pDD;
    }

This will only end if erase happens to set the iterator to end() when it 
invalidates it. In such a case, it will never clear most of the memory used.

Proposal:
void World::PurgeDebugDrawing()
{
        DebugDrawIterator itdd = _debugDrawItems.begin();
        DebugDrawIterator itend = _debugDrawItems.begin();
    for ( ;itdd != itend; ++itdd)
    {
        DebugDrawBase* pDD = (*itdd);
        delete pDD;

    }
        _debugDrawItems.clear();
}

Original issue reported on code.google.com by LoveOver...@gmail.com on 25 Mar 2014 at 11:49

[GoogleCodeExporter]

That's what every C++ implementation I've ever seen actually does, though. 

To be more in line with the spec, I'd just change the one line to:

itdd = _debugDrawItems.erase(itdd);

Original comment by lieseg...@gmail.com on 27 Mar 2014 at 3:32

[GoogleCodeExporter]

That will work!

The reason the STL is designated that way is because it's entirely possible for 
the vector to shrink_to_fit after each erase, which will cause iterators on the 
end to invalidate. Plus, if you switched to boost::stable_vector or a similar 
underlying type, suddenly the assumptions you had might fail.

Original comment by LoveOver...@gmail.com on 27 Mar 2014 at 3:39

[GoogleCodeExporter]

Upon further inspection, this seems to be a recurring issue (and your one line 
change seems to be the driving solution to it in Angel2D).

Actor::~Actor()
{
    StringSet::iterator it = _tags.begin();
    while (it != _tags.end())
    {
        String tag = *it;
        it++;
        Untag(tag);
    }

    Actor::_nameList.erase(_name);
}

For example, will call Untag, which erases from the container, causing faults 
if the pointer is then incremented past the end of the container. ccpcheck 
caught this, but if we restructure it a little:

Actor::~Actor()
{
    StringSet::iterator itend = _tags.end();
    for (StringSet::iterator it = _tags.begin(); it != itend; ++it)
    {
        Untag(*it);
    }

    Actor::_nameList.erase(_name);
}

Now Visual Studio and gdb will assert on it; the temporary and increment only 
serve to mask the issue from casual observation without actually solving it; 
erasing an iterator in a vector is mandated to invalidate pointers after the 
point of erasure.

http://kera.name/articles/2011/06/iterator-invalidation-rules/

Just a heads up, found it due to a memory access violation.

Original comment by LoveOver...@gmail.com on 9 Apr 2014 at 6:24