0

I have some piece of code that runs many times and suddenly reports an access violation.

for(std::list<Projectile>::iterator it = projectiles.begin(); it != projectiles.end();) {
    bool finished = ...

    // try to print address of collides(), prints always 1
    std::cout << &Projectile::collides << std::endl; 
    // here it crashes:
    if(it->collides(&hero)) { 
        std::cout << "Projectile hits " << hero.getName() << std::endl;
        finished = it->apply(&hero);
    }

    // ...
    if(finished) {
        it = projectiles.erase(it);
    } else {
        ++it;
    }
}

So VS debug stacktrace says that in line if(it->collides(&hero)) { the program tries to call a method at cdcdcdcd() which causes the access violation.

it, *it and hero are valid objects according to VS.

So I assume that cdcdcdcd() should actually be collides(). Since collides is a non-virtual method its address should basically not change or?

The thing is that the method collides() is executed several times before successfully, but suddenly it does not work anymore.

Can it be that the address is changed? Have I overwritten it?

Please help me! Also I appreciate information on anything that is not fine with this code :)

jsf
  • 913
  • 3
  • 14
  • 22
  • I think you've posted too small a snippet. There doesn't seem anything obviously wrong with this code, and the real problem likely lies elsewhere. Can you create an [SSCCE](http://sscce.org)? – Greg Hewgill Dec 24 '11 at 18:20
  • Often times problems like this occur because of a lack of locking. Any access to this collection should be locked, otherwise one thread may clear it, while another is iterating. In the end, the symptom is hard to reproduce, because it is a "timing issue." – reuscam Apr 06 '15 at 18:43

2 Answers2

2

0xcdcdcdcd is a fill pattern used by the Win32 Debug CRT Heap; assuming you're running in a debugger on Windows, this may be a significant clue. There's a decent explanation of the heap fill patterns here.

My guess is you're somehow either invalidating the iterator elsewhere, or have some other buffer overflow or dangling pointer or other issue.

Application Verifier may help with diagnosing this. You may also want to look into the other things mentioned in the question How to debug heap corruption errors? or some of the techniques from Any reason to overload global new and delete? (disclaimer: at the moment I have the top-rated answer on both).

If your STL library has a debugging feature it may help ferret this out as well. Metrowerks (now Freescale) Codewarrior has the define _MSL_DEBUG, for example, that can be used to build a version of the standard libraries (including std::list) which will detect common issues like iterator invalidation (at some runtime cost).

It looks like Visual Studio has debug iterator support which might fit the bill, if you're using that.

Community
  • 1
  • 1
leander
  • 8,527
  • 1
  • 30
  • 43
  • The error is gone for now ... I do not know whether what I did to fix it. I do not even know if I fixed it. Your answer was very helpful though. I think i have to make my code more solid and less prone to memory errors. – jsf Dec 25 '11 at 07:37
0

Your termination condition seems wrong [EDIT: actually it looks correct, even if it still scares me. See comments.]. When finished becomes true, you erase the current projectile and then set the iterator to ... something. There is no reason to think that the loop will terminate at this point. It's is likely that it will eventually either start pointing outside of the list, or to an otherwise invalid object (which your debugger will not always flag as such).

When you erase the projectile, you should just explicitly leave the loop using "break".

Adrian Ratnapala
  • 5,485
  • 2
  • 29
  • 39
  • afaik erase() returns an iterator to the next item in the list after the erased one. If the erased item was the last in the list then the end()-iterator is returned. Or am I wrong? – jsf Dec 24 '11 at 18:27
  • @user1104631: that's correct: "The return value is an iterator to the element immediately following the one that was erased." http://www.sgi.com/tech/stl/Sequence.html – leander Dec 24 '11 at 18:55
  • Ok I take it back. The condition at the end is correct. Though I still think he should use an explicit break, I no longer have a precise reason for it. – Adrian Ratnapala Dec 25 '11 at 19:47
  • The reason is obvious. Either the loop is finished or it is not. Continuing to iterate is asking for trouble which is precisely what the OP got. If the iterator returned by `erase` is not at the end of the list then the loop might erase more elements which is probably unexpected. If that behaviour _was_ expected then `finished` was an extremely poor choice of variable name. – Troubadour Dec 26 '11 at 10:13