Fix invalid iterator handling when deleting RequestGroup

src/IndexedList.h

@@ -158,7 +158,7 @@ public:
   // Removes |key| from the list. If the element is not found, this
   // function fails. This function returns true if it
   // succeeds. Complexity: O(N)
-  bool erase(KeyType key)
+  bool remove(KeyType key)
   {
     typename IndexType::iterator i = index_.find(key);
     if(i == index_.end()) {
@@ -175,6 +175,16 @@ public:
     return true;
   }
 
+  // Removes element pointed by iterator |k| from the list. If the
+  // iterator must be valid. This function returns the iterator
+  // pointing to the element following the erased element. Complexity:
+  // O(N)
+  typename SeqType::iterator erase(typename SeqType::iterator k)
+  {
+    index_.erase((*k).first);
+    return seq_.erase(k);
+  }
+
   // Removes element at the front of the list. If the list is empty,
   // this function fails. This function returns true if it
   // succeeds. Complexity: O(logN)

src/RequestGroupMan.cc

@@ -207,7 +207,7 @@ size_t RequestGroupMan::changeReservedGroupPosition
 
 bool RequestGroupMan::removeReservedGroup(a2_gid_t gid)
 {
-  return reservedGroups_.erase(gid);
+  return reservedGroups_.remove(gid);
 }
 
 namespace {
@@ -440,9 +440,8 @@ void RequestGroupMan::removeStoppedGroup(DownloadEngine* e)
         eoi = requestGroups_.end(); i != eoi;) {
     const SharedHandle<RequestGroup>& rg = (*i).second;
     if(rg->getNumCommand() == 0) {
-      ++i;
+      i = requestGroups_.erase(i);
-      requestGroups_.erase(rg->getGID());
+      eoi = requestGroups_.end();
-      // rg is invalid here.
     } else {
       ++i;
     }
@@ -875,7 +874,7 @@ RequestGroupMan::findDownloadResult(a2_gid_t gid) const
 
 bool RequestGroupMan::removeDownloadResult(a2_gid_t gid)
 {
-  return downloadResults_.erase(gid);
+  return downloadResults_.remove(gid);
 }
 
 void RequestGroupMan::addDownloadResult(const SharedHandle<DownloadResult>& dr)

test/IndexedListTest.cc

@@ -17,6 +17,7 @@ class IndexedListTest:public CppUnit::TestFixture {
   CPPUNIT_TEST_SUITE(IndexedListTest);
   CPPUNIT_TEST(testPushBack);
   CPPUNIT_TEST(testPushFront);
+  CPPUNIT_TEST(testRemove);
   CPPUNIT_TEST(testErase);
   CPPUNIT_TEST(testPopFront);
   CPPUNIT_TEST(testMove);
@@ -30,6 +31,7 @@ public:
 
   void testPushBack();
   void testPushFront();
+  void testRemove();
   void testErase();
   void testPopFront();
   void testMove();
@@ -74,7 +76,7 @@ void IndexedListTest::testPushFront()
   }
 }
 
-void IndexedListTest::testErase()
+void IndexedListTest::testRemove()
 {
   int a[] = {1,2,3,4,5};
   IndexedList<int, int*> list;
@@ -82,7 +84,7 @@ void IndexedListTest::testErase()
     list.push_back(i, &a[i]);
   }
   for(int i = 0; i < 5; ++i) {
-    CPPUNIT_ASSERT(list.erase(i));
+    CPPUNIT_ASSERT(list.remove(i));
     CPPUNIT_ASSERT_EQUAL((size_t)5-i-1, list.size());
     for(int j = i+1; j < 5; ++j) {
       CPPUNIT_ASSERT_EQUAL(a[j], *list.get(j));
@@ -90,6 +92,27 @@ void IndexedListTest::testErase()
   }
 }
 
+void IndexedListTest::testErase()
+{
+  int a[] = {1,2,3,4,5};
+  IndexedList<int, int*> list;
+  for(int i = 0; i < 5; ++i) {
+    list.push_back(i, &a[i]);
+  }
+  int* p = a;
+  for(IndexedList<int, int*>::SeqType::iterator i = list.begin();
+      i != list.end();) {
+    i = list.erase(i);
+    CPPUNIT_ASSERT_EQUAL((size_t)(std::distance(i, list.end())), list.size());
+
+    int* pp = ++p;
+    for(IndexedList<int, int*>::SeqType::iterator j = list.begin();
+        j != list.end(); ++j, ++pp) {
+      CPPUNIT_ASSERT_EQUAL(*pp, *(*j).second);
+    }
+  }
+}
+
 void IndexedListTest::testPopFront()
 {
   int a[] = {1,2,3,4,5};