Sign in to follow this  
MeshGearFox

[C++] Crash when trying to delete entries from a list.

Recommended Posts

MeshGearFox    158
This may or may not be related to the issue the guy was having in the post about deleting objects from a vector; however a quick glance through that made me think his issue was iterators getting invalidated, and I'm under the impression that list iterators should NOT be invalidated by erasing elements from a list. Anyway, the purpose of this function is to iterator through two lists of type of Instance, looking for Instance objects which have been flagged for deletion (which the method deletion() is used to indicate). However, the function will cause the program to crash if it has to delete anything. Here's the code:
int Level::cull() {
    list<Instance>::iterator instIter;
 
    int count = 0;
 
    // Cull instance in layers 1 through 2.
    // I have no idea why I'd want to delete floor or overlay tiles.
    for(int x = 1; x < 3; x++) {
        instIter = instances[x].begin();
 
        while(instIter != instances[x].end()) {
            if(instIter->deletion()) {
                // unReg the instance. This should've already been done else.
                if(instIter->acquireLevel()->collisionHandler.unReg(&(*instIter)) != -1) {
                    printf("Instance %s %i not unregistered prior to cull cycle.\n", instIter->name().c_str(), instIter->ID());
                }
 
                // Remove the instance.
                instances[x].erase(instIter);
                count++;
            }
            instIter++;
        }
    }
    return count;
}

What I know: * The *exact* nature of the crash is as such: After an item marked for deletion is found, that object is deleted, and count is increased. The if statement exits, and the instIter iterator is increased. The while loop loops back around and attempts to re-enter the if statement. It APPEARS that performing if(instIter->deletion()) after a successful deletion is what causes the crash. * The actual delete code seems to work. I've tested it by shorting the loop out after one deletion. * What I'm guessing is happening is that instIter gets pointing to something that's not a valid list entry and isn't instances[x].end(), either.

Share this post


Link to post
Share on other sites
DevFred    840
Can you change the following two lines and tell us if that works for you?

while(instIter != instances[x].end()) {
if(instIter->deletion()) {
// unReg the instance. This should've already been done else.
if(instIter->acquireLevel()->collisionHandler.unReg(&(*instIter)) != -1) {
printf("Instance %s %i not unregistered prior to cull cycle.\n", instIter->name().c_str(), instIter->ID());
}

// Remove the instance.
instances[x].erase(instIter++); // change #1
count++;
}
else ++instIter; // change #2
}

Share this post


Link to post
Share on other sites
MeshGearFox    158
Thanks, it did!

I'd tried putting the else statement before instIter++ before, as I'd wondered if deleting would move the instIter forward on its own, but it didn't do anything by itself. So, why DO these changes make it work?

Share this post


Link to post
Share on other sites
DevFred    840
When you say instances[x].erase(instIter), the iterator is invalidated, so you are not allowed to increase it later. Here is where postfix ++ comes in: it increases the iterator while it is still valid, and it returns a copy of the iterator prior to the increment.

For details, see "Effective STL" by Scott Meyers.

Share this post


Link to post
Share on other sites
DevFred    840
An alternative to instances[x].erase(instIter++); that might be less confusing is instIter = instances[x].erase(instIter);.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this