fw: NccStrategy always sets pitEntryEntry->maxInterval

Change-Id: I675745cbd40e8d9c6651e6a728deb194ff37d189
refs: #1853
diff --git a/daemon/fw/ncc-strategy.cpp b/daemon/fw/ncc-strategy.cpp
index 838dd9a..fcabe7a 100644
--- a/daemon/fw/ncc-strategy.cpp
+++ b/daemon/fw/ncc-strategy.cpp
@@ -100,6 +100,12 @@
     pitEntryInfo->maxInterval = std::max(time::microseconds(1),
       time::microseconds((2 * deferRange.count() + nUpstreams - 1) / nUpstreams));
   }
+  else {
+    // Normally, maxInterval is unused if there aren't any face beyond best and previousBest.
+    // However, in case FIB entry gains a new nexthop before doPropagate executes (bug 1853),
+    // this maxInterval would be used to determine when the next doPropagate would happen.
+    pitEntryInfo->maxInterval = deferFirst;
+  }
   pitEntryInfo->propagateTimer = scheduler::schedule(deferFirst,
     bind(&NccStrategy::doPropagate, this,
          weak_ptr<pit::Entry>(pitEntry), weak_ptr<fib::Entry>(fibEntry)));
@@ -143,8 +149,8 @@
   }
 
   if (isForwarded) {
-    boost::random::uniform_int_distribution<time::nanoseconds::rep> dist(
-      0, pitEntryInfo->maxInterval.count() - 1);
+    boost::random::uniform_int_distribution<time::nanoseconds::rep> dist(0,
+      pitEntryInfo->maxInterval.count() - 1);
     time::nanoseconds deferNext = time::nanoseconds(dist(getGlobalRng()));
     pitEntryInfo->propagateTimer = scheduler::schedule(deferNext,
     bind(&NccStrategy::doPropagate, this,
diff --git a/daemon/fw/ncc-strategy.hpp b/daemon/fw/ncc-strategy.hpp
index f78b840..6c284a9 100644
--- a/daemon/fw/ncc-strategy.hpp
+++ b/daemon/fw/ncc-strategy.hpp
@@ -99,8 +99,11 @@
 
   public:
     bool isNewInterest;
+    /// timer that expires when best face does not respond within predicted time
     EventId bestFaceTimeout;
+    /// timer for propagating to another face
     EventId propagateTimer;
+    /// maximum interval between forwarding to two nexthops except best and previous
     time::microseconds maxInterval;
   };
 
diff --git a/tests/daemon/fw/ncc-strategy.cpp b/tests/daemon/fw/ncc-strategy.cpp
index 9e1f7bc..a6229f5 100644
--- a/tests/daemon/fw/ncc-strategy.cpp
+++ b/tests/daemon/fw/ncc-strategy.cpp
@@ -104,6 +104,63 @@
   BOOST_CHECK_EQUAL(strategy->m_sendInterestHistory[2].get<1>(), face2);
 }
 
+BOOST_AUTO_TEST_CASE(Bug1853)
+{
+  LimitedIo limitedIo;
+  Forwarder forwarder;
+  typedef StrategyTester<fw::NccStrategy> NccStrategyTester;
+  shared_ptr<NccStrategyTester> strategy = make_shared<NccStrategyTester>(ref(forwarder));
+  strategy->onAction += bind(&LimitedIo::afterOp, &limitedIo);
+
+  shared_ptr<DummyFace> face1 = make_shared<DummyFace>();
+  shared_ptr<DummyFace> face2 = make_shared<DummyFace>();
+  shared_ptr<DummyFace> face3 = make_shared<DummyFace>();
+  forwarder.addFace(face1);
+  forwarder.addFace(face2);
+  forwarder.addFace(face3);
+
+  Fib& fib = forwarder.getFib();
+  shared_ptr<fib::Entry> fibEntry = fib.insert(Name()).first;
+  fibEntry->addNextHop(face1, 10);
+
+  StrategyChoice& strategyChoice = forwarder.getStrategyChoice();
+  strategyChoice.install(strategy);
+  strategyChoice.insert(Name(), strategy->getName());
+
+  Pit& pit = forwarder.getPit();
+
+  // first Interest: strategy follows routing and forwards to face1
+  shared_ptr<Interest> interest1 = makeInterest("ndn:/nztwIvHX/%00");
+  interest1->setInterestLifetime(time::milliseconds(8000));
+  shared_ptr<pit::Entry> pitEntry1 = pit.insert(*interest1).first;
+
+  pitEntry1->insertOrUpdateInRecord(face3, *interest1);
+  strategy->afterReceiveInterest(*face3, *interest1, fibEntry, pitEntry1);
+
+  limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(1));
+  BOOST_REQUIRE_EQUAL(strategy->m_sendInterestHistory.size(), 1);
+  BOOST_CHECK_EQUAL(strategy->m_sendInterestHistory[0].get<1>(), face1);
+
+  // face1 responds
+  shared_ptr<Data> data1 = makeData("ndn:/nztwIvHX/%00");
+  strategy->beforeSatisfyPendingInterest(pitEntry1, *face1, *data1);
+  limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(500));
+
+  // second Interest: bestFace is face1, nUpstreams becomes 0,
+  // therefore pitEntryInfo->maxInterval cannot be calculated from deferRange and nUpstreams
+  shared_ptr<Interest> interest2 = makeInterest("ndn:/nztwIvHX/%01");
+  interest2->setInterestLifetime(time::milliseconds(8000));
+  shared_ptr<pit::Entry> pitEntry2 = pit.insert(*interest2).first;
+
+  pitEntry2->insertOrUpdateInRecord(face3, *interest2);
+  strategy->afterReceiveInterest(*face3, *interest2, fibEntry, pitEntry2);
+
+  // FIB entry is changed before doPropagate executes
+  fibEntry->addNextHop(face2, 20);
+  limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(1000));// should not crash
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()
 
 } // namespace tests