Crash while erasing a member in a vector

[eeykay]

Hello,

I am facing a starnge problem while erasing the last member in a
vector. I am using VC++ .NET 2002 complier. I have vector of
CComPtr<..> (irrelevant here), and then I iterate over the vector. If
it is the iterator, then I remove the element from the vector using
vecObjects.erase(it). It works fine till the last element. While
removing the last element it throws exception and fails. But the same
vecObject.clear() works with out any problem. Can somebody there,
please help me to indentify this problem and solution to this

Thanks in advance.
EK

 

[E. Mark Ping]

I am facing a starnge problem while erasing the last member in a
vector. I am using VC++ .NET 2002 complier. I have vector of
CComPtr<..> (irrelevant here), and then I iterate over the vector. If
it is the iterator, then I remove the element from the vector using
vecObjects.erase(it). It works fine till the last element. While
removing the last element it throws exception and fails. But the same
vecObject.clear() works with out any problem. Can somebody there,
please help me to indentify this problem and solution to this

You are most likely going off the end of the vector and erasing an
invalid iterator. Please post a code snippet that can compile and
you'll get much better responses.

 

[Jordan]

Yes, the 'last' position in a vector is one beyond the last element.

If you are progressing through your vectory array, check if the address
of the iterator has reached the address of the last position. See the
code example below.

vector<int>::iterator iter = 0;
for( iter = vecList.begin(); iter != vecList.end(); iter++ )
{
delete *iter;
}

 

[Howard]

Yes, the 'last' position in a vector is one beyond the last element.

If you are progressing through your vectory array, check if the address
of the iterator has reached the address of the last position. See the
code example below.

vector<int>::iterator iter = 0;
for( iter = vecList.begin(); iter != vecList.end(); iter++ )
{
delete *iter;
}

That's how to delete dynamically allocated objects from a vector, but the
iterator you've declared refers to a vector that does not have pointers, it
has just int's. You don't want to delete an int, or anything that was not
created via new. (I know, it's just an example, but it's a bad example.
)

The OP asked about using erase() to remove items from a vector. That's
different from deleting the dynamically allocated data pointed to by
pointers stored in the vector. And without seeing his code, we can't say
_for_sure_ what the problem is. My "guess" is that clear() will work fine,
but it's just a guess at this point. Only if he also wants to delete
dynamically allocated objects will he first want to have a loop like the one
you've shown.

-Howard

 

[BobR]

(e-mail address removed) wrote in message

Hello,

I am facing a starnge problem while erasing the last member in a
vector. I am using VC++ .NET 2002 complier. I have vector of
CComPtr<..> (irrelevant here), and then I iterate over the vector. If
it is the iterator, then I remove the element from the vector using
vecObjects.erase(it). It works fine till the last element. While
removing the last element it throws exception and fails. But the same
vecObject.clear() works with out any problem. Can somebody there,
please help me to indentify this problem and solution to this
Thanks in advance.
EK

You forgot to show the code! Are you trying to do this in a loop?

If you do:
vecObjects.erase(it);
....then iterator 'it' may no longer be valid [1]. Reset it.

... It works fine till the last element.

Use:
vecObjects.pop_back(); // Removes the last element.

[1] " A vector's iterators are invalidated when its memory is reallocated.
Additionally, inserting or deleting an element in the middle of a vector
invalidates all iterators that point to elements following the insertion or
deletion point. ..."

 

[Heinz Ozwirk]

Hello,

I am facing a starnge problem while erasing the last member in a
vector. I am using VC++ .NET 2002 complier. I have vector of
CComPtr<..> (irrelevant here),

Perhaps it is irrelevant for your problem, but you should not put instances
of CComPtr into C++ containers. Some of these containers (or some
implementations of them) do not work well with objects overloading
operator&.

and then I iterate over the vector. If
it is the iterator, then I remove the element from the vector using
vecObjects.erase(it).

When you iterate through a container and try to erase its last element, make
sure not to increment the iterator after erasing. Actually you should never
increment an iterator after erasing the element it refers to. For all
containers, all iterators refering to the erased element become invalid. So
don't do something like this

for (SomeContainer::iterator it = myContainer.begin(); it !=
myContainer.end(); ++it)
{
if (RemoveThisElement(*it)) myContainer.erase(it);
}

If SomeContainer is an std::vector, this seems to work until you try to
erase the last element, but actually it will not examine those elements
imediately following an element, that will be removed. Now if you erase the
last element, end() will change (in some implementations end() will become
equal to the iterator for the erased elemet) and due to the final increment
the test for equality to end() will fail and the loop will continue until it
reaches the end of allocated memory. (Or it will behave entirely different,
after all that's the problem with undefined behaviour.)

HTH
Heinz

 

[E. Mark Ping]

for (SomeContainer::iterator it = myContainer.begin(); it !=
myContainer.end(); ++it)
{
if (RemoveThisElement(*it)) myContainer.erase(it);
}

If SomeContainer is an std::vector, this seems to work until you try to
erase the last element, but actually it will not examine those elements
imediately following an element, that will be removed.

This is actually worse, because as you remove the elements you
invalidate your iterator, so your "++it" is a very bad thing.

Better is:

vector<int> container;
vector<int>::iterator iter;
for (iter=container.begin(); iter != container.end(); )
{
if (RemoveThisElement(*iter)) {
iter = container.erase(iter);
} else {
++iter
}
}

Note the assignment of "iter" from erase, and the lack of ++iter in
the for loop. Either the iter is manually incremented or the erase
operation leaves it pointing to the next position.

 

[eeykay]

Hello All,

Thanks for the huge response, I have got. I am yet to try the
suggestions. In the mean time I am adding the code snippet here for
better clarity of the question I have asked.

Though I cannot reproduce the code, it looks similar to this.

I have a vector vecObjects with CComPtr<Interfaces I..>. Now I have to
check the contents for this vector for some conditions. Hence I iterate
the container as follows.

std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
vecObjects.erase(it); // This causes the crash with the last
element
}
++it;
}

// At the same time the following code works

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
if(vecObjects.size() == 1)
vecObjects.clear();
else
vecObjects.erase(it);
}
++it;
}

Thank you for all the help.
EK

 

[E. Mark Ping]

std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
vecObjects.erase(it); // This causes the crash with the last
element
}
++it;
}

Yeah, this is broken.

// At the same time the following code works

No it doesn't.

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
if(vecObjects.size() == 1)
vecObjects.clear();
else
vecObjects.erase(it);
}
++it;
}

the 'erase' invalidates all the iterators. You can't cache the 'end'
iterator. Furthermore, you need to use the return value of erase,
since you just invalidated 'it' when you erased it (and hence
incrementing it is unsafe).

 

[Niklas Norrthon]

Hello,

I am facing a starnge problem while erasing the last member in a
vector. I am using VC++ .NET 2002 complier. I have vector of
CComPtr<..> (irrelevant here), and then I iterate over the vector. If
it is the iterator, then I remove the element from the vector using
vecObjects.erase(it). It works fine till the last element. While
removing the last element it throws exception and fails. But the same
vecObject.clear() works with out any problem. Can somebody there,
please help me to indentify this problem and solution to this

Difficult to say without seeing your code. One possibility is that
you somewhere use an iterator which was earlier invalidated by
the call to vector<CComPtr>::erase. When an element in a vector
is erased, every iterator to that element and all elements following
it are invalidated.

If that is your problem, it could be possible that you could
get around it by iterating from end towards begin instead.

/Niklas Norrthon

 

[Zara]

Hello All,
(...)

std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
vecObjects.erase(it); // This causes the crash with the last
element
}
++it;
}

(...)

From the code you show us, I think the best soultion is to use the
algorithm std::remove_if using as predicte a functor written around
IsValid.

std::vector<...>::iterator lastPosition=
std::remove_if(vecObjectsbegin(),vecObjects.end(),wrapIsValid());
vecObjects.erase(lastPosition,vecObjects.end());

Or something similar.

Best regards,

Zara

 

[Niklas Norrthon]

Hello All,

Thanks for the huge response, I have got. I am yet to try the
suggestions. In the mean time I am adding the code snippet here for
better clarity of the question I have asked.

Though I cannot reproduce the code, it looks similar to this.

I have a vector vecObjects with CComPtr<Interfaces I..>. Now I have to
check the contents for this vector for some conditions. Hence I iterate
the container as follows.

std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()

for(;it != itEnd; ++it); // I tried post increment also
{
// Check the iterator for some conditions
if (! IsValid(*it))
{
vecObjects.erase(it); // This causes the crash with the last
element
}
++it;
}

Consider including <algorithm> and <functional> and then do this
instead:

using std::not1;
using std:tr_fun;
using std::remove_if;

typedef std::vector<CComPtr<I...> >::iterator Iter;

Iter it = remove_if(vecObjects.begin(), vecObjects.end(),
not1(ptr_fun(IsValid)));
vecObjects.erase(it, vecObjects.end());

/Niklas Norrthon