commit ebe156c92f9907b37068b52c2dba351af78f5a09
author Ashlesh Gawande <agawande@memphis.edu> Mon Nov 12 16:27:49 2018 -0600
committer Alexander Afanasyev <aa@cs.fiu.edu> Wed Dec 26 10:03:05 2018 -0800
tree 895eba3307d7967d4cd84a9760bf3e2744161dd4
parent 19b87de693a85f11e29efab20a71f794cadec83e

util: fix SegmentFetcher undefined behavior caused by uncanceled pending interest

Change-Id: I4ca80ad00fc52024f35908b21d7c42bffc8a5396
Refs: #4770

ndn-cxx/util/segment-fetcher.cpp

diff --git a/ndn-cxx/util/segment-fetcher.cpp b/ndn-cxx/util/segment-fetcher.cpp
index 01419c1..71799dd 100644
--- a/ndn-cxx/util/segment-fetcher.cpp
+++ b/ndn-cxx/util/segment-fetcher.cpp
@@ -229,8 +229,6 @@
   if (shouldStop(weakSelf))
     return;
 
-  afterSegmentReceived(data);
-
   BOOST_ASSERT(m_nSegmentsInFlight > 0);
   m_nSegmentsInFlight--;
 
@@ -250,6 +248,12 @@
     pendingSegmentIt = m_pendingSegments.begin();
   }
 
+  if (pendingSegmentIt == m_pendingSegments.end()) {
+    return;
+  }
+
+  afterSegmentReceived(data);
+
   // Cancel timeout event
   m_scheduler.cancelEvent(pendingSegmentIt->second.timeoutEvent);
   pendingSegmentIt->second.timeoutEvent = nullptr;
@@ -479,6 +483,7 @@
   BOOST_ASSERT(pendingSegmentIt != m_pendingSegments.end());
   BOOST_ASSERT(pendingSegmentIt->second.state == SegmentState::InRetxQueue);
   pendingSegmentIt->second.state = SegmentState::Retransmitted;
+  m_face.removePendingInterest(pendingSegmentIt->second.id);
   pendingSegmentIt->second.id = pendingInterest;
   pendingSegmentIt->second.timeoutEvent = timeoutEvent;
 }

tests/unit/util/segment-fetcher.t.cpp

diff --git a/tests/unit/util/segment-fetcher.t.cpp b/tests/unit/util/segment-fetcher.t.cpp
index 22aa0b7..f870030 100644
--- a/tests/unit/util/segment-fetcher.t.cpp
+++ b/tests/unit/util/segment-fetcher.t.cpp
@@ -855,6 +855,28 @@
   BOOST_CHECK_EQUAL(nAfterSegmentTimedOut, 2);
 }
 
+BOOST_AUTO_TEST_CASE(UncanceledPendingInterestBug) // Bug #4770
+{
+  DummyValidator acceptValidator;
+  auto fetcher = SegmentFetcher::start(face, Interest("/hello/world"), acceptValidator);
+  connectSignals(fetcher);
+
+  // Fetcher will send the first interest immediately
+  // and the second interest after 1 second by default
+  advanceClocks(1100_ms);
+
+  // Face will give data to the fetcher twice if the first interest is not canceled.
+  // isFinal=false to keep fetcher alive so that it can receive the second data.
+  face.receive(*makeDataSegment("/hello/world/version0", 0, false));
+
+  advanceClocks(1100_ms);
+
+  face.receive(*makeDataSegment("/hello/world/version0", 1, true));
+
+  BOOST_CHECK_EQUAL(nErrors, 0);
+  BOOST_CHECK_EQUAL(nCompletions, 1);
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestSegmentFetcher
 BOOST_AUTO_TEST_SUITE_END() // Util