Segmentation fault while iterating through and erasing some elements from std::map?

Question

I am using several std::map type data containers which are the following:

    std::map<int, std::vector< cv::Point > > mapGoodContours;
    std::map<int, EllipseProperties> ellipsePropertiesMap;
    std::map<int, float> m_markerRadiusMap;
    std::map<int,cv::Point2f> markers;//This is part of another class

I iterate through these containers and erase some elements after those elements meet certain condition as shown in the following code.

    auto radiusMapCounter = m_markerRadiusMap.begin();
    auto markerCounter = frames.markers.begin();
    auto goodContoursCounter = mapGoodContours.begin();

    if(m_markerRadiusMap.size()==ellipsePropertiesMap.size() && frames.markers.size()==ellipsePropertiesMap.size()
            && mapGoodContours.size()==ellipsePropertiesMap.size())
    {
        for(auto ellipsePropertyCounter = ellipsePropertiesMap.begin(); ellipsePropertyCounter != ellipsePropertiesMap.end(); ellipsePropertyCounter++)
        {

            float upperLimit = (float)m_upperFactor*(float)ellipsePropertyCounter->second.radius;
            float lowerLimit = (float)m_lowerFactor*(float)ellipsePropertyCounter->second.radius;
            if(ellipsePropertyCounter->second.minDistanceFromOtherEllipse>upperLimit
                        || ellipsePropertyCounter->second.minDistanceFromOtherEllipse<lowerLimit)
            {
                 ellipsePropertiesMap.erase(ellipsePropertyCounter);
                 m_markerRadiusMap.erase(radiusMapCounter);
                 frames.markers.erase(markerCounter);
                 mapGoodContours.erase(goodContoursCounter);
            }
            else
            {
                 smallContours.push_back(goodContoursCounter->second);
            }

            radiusMapCounter++;
            markerCounter++;
            goodContoursCounter++;
        }
    }

I am baffled to find that sometimes I have segmentation faults as shown in the image. The fault specifically points to the line with the code radiusMapCounter++;

What am I doing wrong?

Possible duplicate of [Iterator invalidation rules](http://stackoverflow.com/questions/6438086/iterator-invalidation-rules) — MikeCAT, Jul 25 '16 at 16:04

Paul · Accepted Answer · 2016-07-25T16:24:45.043

You cannot increment iterator after you erased the element it points to from container. Create a copy of iterator, increment it, delete element via copy.

If you are using C++11 or later, std::map::erase(...) returns the iterator following the last removed element, so you can use it instead of incrementing invalid one. No need to create a copy of iterator to use in erase in this case.

for(auto ellipsePropertyCounter = ellipsePropertiesMap.begin();
    ellipsePropertyCounter != ellipsePropertiesMap.end();
    /* do not increment iterator here */)
{

    float upperLimit = (float)m_upperFactor*(float)ellipsePropertyCounter->second.radius;
    float lowerLimit = (float)m_lowerFactor*(float)ellipsePropertyCounter->second.radius;
    if(ellipsePropertyCounter->second.minDistanceFromOtherEllipse>upperLimit
                || ellipsePropertyCounter->second.minDistanceFromOtherEllipse<lowerLimit)
    {
         ellipsePropertyCounter = ellipsePropertiesMap.erase(ellipsePropertyCounter);
         radiusMapCounter = m_markerRadiusMap.erase(radiusMapCounter);
         markerCounter = frames.markers.erase(markerCounter);
         goodContoursCounter = mapGoodContours.erase(goodContoursCounter);
    }
    else
    {
         smallContours.push_back(goodContoursCounter->second);

         radiusMapCounter++;
         markerCounter++;
         goodContoursCounter++;
         ellipsePropertyCounter++; // increment loop iterator only in 'else' case
    }
}

score 0 · Answer 2 · answered Jul 25 '16 at 18:12

Here is a C++17 solution.

First indexer, which is C++14 and lets you create and unpack index packs inline without special purpose helpers at point of use:

template<class=void,std::size_t...Is>
void indexer( std::index_sequence<Is>... ) {
  return [](auto&& f) {
    using discard=int[];
    (void)discard{0,((
      f( std::integral_constant<std::size_t, Is>{} )
    ),void(),0)...};
  };
}
template<std::size_t N>
void indexer() {
  return indexer( std::make_index_sequence<N>{} );
}

Now, erase_if, which should be SFINAE restricted to only work on associative (and maybe other node-based) containers:

template<class Test, class...Maps>
bool erase_if( Test&& test, Maps&&...maps) {
  using std::begin; using std::end;
  std::tuple< decltype(begin(maps))... > its;
  std::tuple< decltype(begin(maps))... > ends;
  auto m_tup = std::tie(maps...);
  auto index = indexer<sizeof...(maps)>;
  index(
    [&](auto i){
      std::get<i>(its) = begin( std::get<i>(m_tup) );
      std::get<i>(ends) = end( std::get<i>(m_tup) );
    }
  );
  while (its != ends) {
    auto deref_then_test = [&test](auto...its) {
      return test( (*its)... );
    };
    if (std::apply(deref_then_test,its)) {
      index(
        [&](auto i){
          std::get<i>(its) = std::get<i>(m_tup).erase(std::get<i>(its));
        }
      );
    } else {
      index(
        [&](auto i){++std::get<i>(its);}
      );
    }
  }
}

this lets you erase from multiple containers based off tests on one of them.

Code not tested, probably contians typos.

Segmentation fault while iterating through and erasing some elements from std::map?

2 Answers2