commit 4b2e6cb32142f5992b58916610a2e7b4f1a679b3
author Junxiao Shi <git@mail1.yoursunny.com> Wed Dec 03 00:51:45 2014 -0700
committer Junxiao Shi <sunnylandh@gmail.com> Wed Dec 03 10:36:27 2014 -0800
tree 6e3dccee6c72beb6c0bcd08999fab2b858a6cf57
parent 7726ae5182f2c127fc41e52d0f069f724e22ce63

table: fix Fib::removeNextHopFromAllEntries invalidating NameTree iterator

refs #2177

Change-Id: I19b8234d94b804273404176d6751a91f082653b7

daemon/table/fib.cpp

diff --git a/daemon/table/fib.cpp b/daemon/table/fib.cpp
index 99a1bf9..c28a695 100644
--- a/daemon/table/fib.cpp
+++ b/daemon/table/fib.cpp
@@ -143,26 +143,21 @@
 void
 Fib::removeNextHopFromAllEntries(shared_ptr<Face> face)
 {
-  shared_ptr<fib::Entry> toErase;
+  std::list<fib::Entry*> toErase;
 
   auto&& enumerable = m_nameTree.fullEnumerate(&predicate_NameTreeEntry_hasFibEntry);
   for (const name_tree::Entry& nte : enumerable) {
-    if (toErase != nullptr) {
-      this->erase(*toErase);
-      toErase.reset();
-    }
-
     shared_ptr<fib::Entry> entry = nte.getFibEntry();
     entry->removeNextHop(face);
     if (!entry->hasNextHops()) {
-      toErase = entry;
-      // entry needs to be erased, but we must wait until next iteration
-      // to erase it because otherwise it would invalidate the iterator
+      toErase.push_back(entry.get());
+      // entry needs to be erased, but we must wait until the enumeration ends,
+      // because otherwise NameTree iterator would be invalidated
     }
   }
 
-  if (toErase != nullptr) {
-    this->erase(*toErase);
+  for (fib::Entry* entry : toErase) {
+    this->erase(*entry);
   }
 }
 

tests/daemon/table/fib.cpp

diff --git a/tests/daemon/table/fib.cpp b/tests/daemon/table/fib.cpp
index 35fcd3e..d75088b 100644
--- a/tests/daemon/table/fib.cpp
+++ b/tests/daemon/table/fib.cpp
@@ -264,6 +264,22 @@
   BOOST_CHECK_EQUAL(entry->getPrefix(), nameEmpty);
 }
 
+BOOST_AUTO_TEST_CASE(RemoveNextHopFromManyEntries)
+{
+  NameTree nameTree(16);
+  Fib fib(nameTree);
+  shared_ptr<Face> face1 = make_shared<DummyFace>();
+
+  for (uint64_t i = 0; i < 300; ++i) {
+    shared_ptr<fib::Entry> entry = fib.insert(Name("/P").appendVersion(i)).first;
+    entry->addNextHop(face1, 0);
+  }
+  BOOST_CHECK_EQUAL(fib.size(), 300);
+
+  fib.removeNextHopFromAllEntries(face1);
+  BOOST_CHECK_EQUAL(fib.size(), 0);
+}
+
 void
 validateFindExactMatch(const Fib& fib, const Name& target)
 {