Vector.erase(Iterator) causes bad memory access

Question

I am trying to do a Z-Index reordering of videoObjects stored in a vector. The plan is to identify the videoObject which is going to be put on the first position of the vector, erase it and then insert it at the first position. Unfortunately the erase() function always causes bad memory access.

Here is my code:

testApp.h:

vector<videoObject> videoObjects;
vector<videoObject>::iterator itVid;

testApp.cpp:

// Get the videoObject which relates to the user event
for(itVid = videoObjects.begin(); itVid != videoObjects.end(); ++itVid) {
  if(videoObjects.at(itVid - videoObjects.begin()).isInside(ofPoint(tcur.getX(), tcur.getY()))) {
   videoObjects.erase(itVid);
  }
}

This should be so simple but I just don't see where I'm taking the wrong turn.

Why `videoObjects.at(itVid - videoObjects.begin())` instead of `(*itVid)`? — James McNellis, May 31 '10 at 14:01
tried, but it causes the following error: no matching function for call to 'std::vector >::at(videoObject&)' — xon1c, May 31 '10 at 14:04
You don't need to use `at()` at all; dereferencing the iterator returns a reference to the pointed-to element. — James McNellis, May 31 '10 at 14:05
Duplicate problem to this question... http://stackoverflow.com/questions/2728551/c-iterators-problem/2728567#2728567 - do we close off same-solution questions? This is the same old erase-invalidates-iterator problem. — AshleysBrain, May 31 '10 at 14:58

score 15 · Accepted Answer · edited Jun 20 '20 at 09:12

15

You should do

itVid = videoObjects.erase(itVid);

Quote from cplusplus.com:

[vector::erase] invalidates all iterator and references to elements after position or first.

Return value: A random access iterator pointing to the new location of the element that followed the last element erased by the function call, which is the vector end if the operation erased the last element in the sequence.

Update: the way you access the current element inside your condition looks rather strange. Also one must avoid incrementing the iterator after erase, as this would skip an element and may cause out-of-bounds errors. Try this:

for(itVid = videoObjects.begin(); itVid != videoObjects.end(); ){
  if(itVid->isInside(ofPoint(tcur.getX(), tcur.getY()))){
    itVid = videoObjects.erase(itVid);
  } else {
    ++itVid;
  }
}

edited Jun 20 '20 at 09:12

Community

1
1

answered May 31 '10 at 13:47

Péter Török

114,404
31
268
329

3

To anyone using Visual C++ 2010: You need to disable iterator debugging to get this to work (basically, iterator debugging is hopelessly broken in this scenario): https://connect.microsoft.com/VisualStudio/feedback/details/557029/ – James McNellis May 31 '10 at 13:57
I've tried: `for(itVid = videoObjects.begin(); itVid != videoObjects.end(); ++itVid){ if(videoObjects.at(itVid - videoObjects.begin()).isInside(ofPoint(tcur.getX(), tcur.getY()))){ itVid = videoObjects.erase(itVid); break; } } but still get a bad memory access... – xon1c May 31 '10 at 13:59
@Péter: I'm greatly appreciating your help. Unfortunately I do still get the bad memmory access :( – xon1c May 31 '10 at 14:19
@xon1c, have you tried debugging? How many times the loop is executed before the failure? Are multiple elements erased, or does the very first erase fail? – Péter Török May 31 '10 at 14:28
@Péter: I have 3 videoObjects in the vector. All 3 are being displayed on screen. By clicking on one, it gets deleted. The bad memory access happens with either one. So the function will always erase only one videoObject but I've tried, it doesn't matter on which index position that object is stored. – xon1c May 31 '10 at 14:38
@xon1c, if the error happens right in the call to `erase`, it may also be a destructor issue. Could you publish the constructor(s) and destructor of `videoObject`? – Péter Török May 31 '10 at 14:46
@xon1c, how about the embedded `videoClip`? Try commenting out that member (and any other non-primitive member fields) and see whether the bug still occurs... – Péter Török May 31 '10 at 15:31
@Péter: I am using the ofVideoPlayer class provided by openframeworks. I've discovered that its constructor has a method `tex.clear();` that causes the exception. After putting that line in a comment, the `erase()` function now works correctly. Hopefully not calling 'tex.clear();` won't bring up any other potential problems. However, thank you very much for your help & patience! – xon1c May 31 '10 at 15:46
@Péter: sorry, I was wrong... still getting bad memory access when I uncomment the videoClip part. :( – xon1c May 31 '10 at 15:51
@xon1c, that at least proves that the culprit is that class. – Péter Török May 31 '10 at 15:56
@Péter: ok, now finally the `videoObject` gets removed properly. The `ofVideoPlayer` uses the class `ofTexture` which also causes problems when the destructor is called. I've removed all lines from both destructors and all works fine now but I'm just waiting for the next error coming up because of this workaround... *sigh* – xon1c May 31 '10 at 16:18

score 3 · Answer 2 · answered May 31 '10 at 19:25

Beware, erasing elements one by one from a vector has quadratic complexity. STL to the rescue!

#include <algorithm>
#include <functional>

videoObjects.erase(
    std::remove_if(
        std::bind2nd(
            std::mem_fun_ref(&videoObject::isInside),
            ofPoint(tcur.getX(), tcur.getY())
        ),
    ),
    videoObjects.end()
);

score 1 · Answer 3 · answered May 31 '10 at 13:46

1

You cannot delete while iterating over the list because the iterator gets invalid. You should use the return iterator of Erase to set it to your current iterator.

answered May 31 '10 at 13:46

RvdK

19,580
4
64
107

score 0 · Answer 4 · answered May 31 '10 at 13:47

0

erase function returns the next valid iterator.

You would have to make a while loop and do something like

iterator = erase(...)

with corresponding checks.

answered May 31 '10 at 13:47

M. Williams

4,945
2
26
27

Vector.erase(Iterator) causes bad memory access

4 Answers4

Linked

Related