c++ – Invalidating an iterator in std :: map

Question:

Many sources like http://www.amse.ru/courses/cpp1/2010.04.07.html say nothing about invalidating iterators in map.

Is the UB code below dangerous?

std::map<Word32, OnlineUser*>::iterator toDelete, it = onlineUsers.begin();
while (it != onlineUsers.end())
    if (it->second) 
    {
        if (curUnixTimeStamp - it->second->lastActiveTime > MAX_NON_ACTIVETIME)
        {
            toDelete = it;
            it++;
            OnlineUser* userToDelete = toDelete->second;
            onlineUsers.erase(toDelete);
            delete userToDelete;
        } else {
            it++;
        }
    }

Unfortunately I have gcc version 4.4.7? doesn't support lambdas or return iterators after erase, so I can't do as stackoverflow advises like it = c.erase (it); Update: I tried to use the erase (remove_if) bundle, but I'm having difficulties with the predicate. I cannot use lambdas because of the compiler version and my predicate requires the use of a local variable. Tried creating a class object

    class Predicate{
public:
    Predicate(Word32 curUnixTimeStamp):_curUnixTimeStamp(curUnixTimeStamp){}
    bool operator()(std::pair<Word32, Sites::OnlineUser*> user){
        return _curUnixTimeStamp - (Word32)user.second->lastActiveTime > (Word32)MAX_NON_ACTIVETIME ;
    }
private:
    Word32 _curUnixTimeStamp;   
};

and use

class Predicate p(curUnixTimeStamp);
onlineUsers.erase(std::remove_if(onlineUsers.begin(), onlineUsers.end(),p));

but a heap of mistakes falls out

/usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_pair.h: In member function ‘std::pair<const unsigned int, Sites::OnlineUser*>& std::pair<const unsigned int, Sites::OnlineUser*>::operator=(const std::pair<const unsigned int, Sites::OnlineUser*>&)’:
/usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_pair.h:68:   instantiated from ‘_FIter std::remove_if(_FIter, _FIter, _Predicate) [with _FIter = std::_Rb_tree_iterator<std::pair<const unsigned int, Sites::OnlineUser*> >, _Predicate = Sites::Predicate]’
sites.hpp:195:   instantiated from here
/usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_pair.h:68: error: non-static const member ‘const unsigned int std::pair<const unsigned int, Sites::OnlineUser*>::first’, can't use default assignment operator
In file included from /usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/algorithm:62,
                 from sites.hpp:13,
                 from banner.cpp:10:
/usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_algo.h: In function ‘_FIter std::remove_if(_FIter, _FIter, _Predicate) [with _FIter = std::_Rb_tree_iterator<std::pair<const unsigned int, Sites::OnlineUser*> >, _Predicate = Sites::Predicate]’:
/usr/lib/gcc/i686-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_algo.h:1161: note: synthesized method ‘std::pair<const unsigned int, Sites::OnlineUser*>& std::pair<const unsigned int, Sites::OnlineUser*>::operator=(const std::pair<const unsigned int, Sites::OnlineUser*>&)’ first required here 
gmake: *** [banner.o] Error 1

Answer:

The code you provided, while using an iterator and deleting records from the container at the same time, is not safe.

Consider your problematic build error line:

onlineUsers.erase(std::remove_if(onlineUsers.begin(), onlineUsers.end(),p));

1) The std::remove and std::remove_if cannot be used with std::set and std::map , since the first field has a const modifier, and the algorithms in their implementation use std::move .
2) Even if you exclude the first item, the record will not be correct, since std::remove_if will already remove all elements approved by the predicate. In this case, the erase will go to erase to the next one after the last deleted one, and this may turn out to be end() .

I propose to solve the problem by simple separation of tasks, namely, first find those who should be deleted, and then delete them:

std::list<Word32> delUserKeys; // Будет хранить ключи кандидатов на удаление.
std::map<Word32, OnlineUser*>::iterator users_it = onlineUsers.begin();
while (users_it != onlineUsers.end())
if (users_it->second)
{
    if (curUnixTimeStamp - users_it->second->lastActiveTime > MAX_NON_ACTIVETIME)
    {
        delUserKeys.push_back(users_it->first);             
    }

    users_it++;
}

std::list<Word32>::iterator delUserKeys_it = delUserKeys.begin();
while (delUserKeys_it != delUserKeys.end()) {
    std::map<Word32, OnlineUser*>::iterator thisDel_it = onlineUsers.find(*delUserKeys_it);
    if (thisDel_it != onlineUsers.end()) {
        delete thisDel_it->second;
        onlineUsers.erase(thisDel_it);
    }
}

delUserKeys.clear();
Scroll to Top