face: fix NDNLP PartialMessage cleanup scheduling

refs #2414

Change-Id: Ida9e47e41c4dde9b3ee0ee7003fc4aaa79ea55df
diff --git a/common.hpp b/common.hpp
index 8316184..e7ed28d 100644
--- a/common.hpp
+++ b/common.hpp
@@ -1,12 +1,12 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014,  Regents of the University of California,
- *                      Arizona Board of Regents,
- *                      Colorado State University,
- *                      University Pierre & Marie Curie, Sorbonne University,
- *                      Washington University in St. Louis,
- *                      Beijing Institute of Technology,
- *                      The University of Memphis
+ * Copyright (c) 2014-2015,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
  *
  * This file is part of NFD (Named Data Networking Forwarding Daemon).
  * See AUTHORS.md for complete list of NFD authors and contributors.
@@ -51,8 +51,11 @@
 
 #include <cstddef>
 #include <list>
+#include <map>
 #include <set>
 #include <queue>
+#include <unordered_map>
+#include <unordered_set>
 #include <vector>
 
 #include <ndn-cxx/common.hpp>
diff --git a/daemon/face/ndnlp-partial-message-store.cpp b/daemon/face/ndnlp-partial-message-store.cpp
index 54a5537..fafeaf0 100644
--- a/daemon/face/ndnlp-partial-message-store.cpp
+++ b/daemon/face/ndnlp-partial-message-store.cpp
@@ -1,11 +1,12 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014  Regents of the University of California,
- *                     Arizona Board of Regents,
- *                     Colorado State University,
- *                     University Pierre & Marie Curie, Sorbonne University,
- *                     Washington University in St. Louis,
- *                     Beijing Institute of Technology
+ * Copyright (c) 2014-2015,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
  *
  * This file is part of NFD (Named Data Networking Forwarding Daemon).
  * See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
  *
  * You should have received a copy of the GNU General Public License along with
  * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
 
 #include "ndnlp-partial-message-store.hpp"
 
@@ -99,38 +100,28 @@
   }
 
   uint64_t messageIdentifier = parsed.m_seq - parsed.m_fragIndex;
-  shared_ptr<PartialMessage> pm = m_partialMessages[messageIdentifier];
-  if (!static_cast<bool>(pm)) {
-    m_partialMessages[messageIdentifier] = pm = make_shared<PartialMessage>();
-  }
+  PartialMessage& pm = m_partialMessages[messageIdentifier];
   this->scheduleCleanup(messageIdentifier, pm);
 
-  pm->add(parsed.m_fragIndex, parsed.m_fragCount, parsed.m_payload);
-  if (pm->isComplete()) {
-    this->onReceive(pm->reassemble());
+  pm.add(parsed.m_fragIndex, parsed.m_fragCount, parsed.m_payload);
+  if (pm.isComplete()) {
+    this->onReceive(pm.reassemble());
     this->cleanup(messageIdentifier);
   }
 }
 
 void
 PartialMessageStore::scheduleCleanup(uint64_t messageIdentifier,
-                                     shared_ptr<PartialMessage> partialMessage)
+                                     PartialMessage& partialMessage)
 {
-  partialMessage->m_expiry = scheduler::schedule(m_idleDuration,
+  partialMessage.expiry = scheduler::schedule(m_idleDuration,
     bind(&PartialMessageStore::cleanup, this, messageIdentifier));
 }
 
 void
 PartialMessageStore::cleanup(uint64_t messageIdentifier)
 {
-  std::map<uint64_t, shared_ptr<PartialMessage> >::iterator it =
-    m_partialMessages.find(messageIdentifier);
-  if (it == m_partialMessages.end()) {
-    return;
-  }
-
-  scheduler::cancel(it->second->m_expiry);
-  m_partialMessages.erase(it);
+  m_partialMessages.erase(messageIdentifier);
 }
 
 } // namespace ndnlp
diff --git a/daemon/face/ndnlp-partial-message-store.hpp b/daemon/face/ndnlp-partial-message-store.hpp
index 4143107..6fbf0f7 100644
--- a/daemon/face/ndnlp-partial-message-store.hpp
+++ b/daemon/face/ndnlp-partial-message-store.hpp
@@ -34,11 +34,21 @@
 
 /** \brief represents a partially received message
  */
-class PartialMessage : noncopyable
+class PartialMessage
 {
 public:
   PartialMessage();
 
+  PartialMessage(const PartialMessage&) = delete;
+
+  PartialMessage&
+  operator=(const PartialMessage&) = delete;
+
+  PartialMessage(PartialMessage&&) = default;
+
+  PartialMessage&
+  operator=(PartialMessage&&) = default;
+
   bool
   add(uint16_t fragIndex, uint16_t fragCount, const Block& payload);
 
@@ -56,7 +66,7 @@
   reassemble();
 
 public:
-  scheduler::EventId m_expiry;
+  scheduler::ScopedEventId expiry;
 
 private:
   size_t m_fragCount;
@@ -84,18 +94,19 @@
   void
   receiveNdnlpData(const Block& pkt);
 
-  /// fires when network layer packet is received
+  /** \brief fires when network layer packet is received
+   */
   signal::Signal<PartialMessageStore, Block> onReceive;
 
 private:
   void
-  scheduleCleanup(uint64_t messageIdentifier, shared_ptr<PartialMessage> partialMessage);
+  scheduleCleanup(uint64_t messageIdentifier, PartialMessage& partialMessage);
 
   void
   cleanup(uint64_t messageIdentifier);
 
 private:
-  std::map<uint64_t, shared_ptr<PartialMessage> > m_partialMessages;
+  std::unordered_map<uint64_t, PartialMessage> m_partialMessages;
 
   time::nanoseconds m_idleDuration;
 };
diff --git a/tests/daemon/face/ndnlp.cpp b/tests/daemon/face/ndnlp.cpp
index c80e4f7..52091d3 100644
--- a/tests/daemon/face/ndnlp.cpp
+++ b/tests/daemon/face/ndnlp.cpp
@@ -66,7 +66,7 @@
   BOOST_CHECK_NE(sb1[1], sb2[0]);
 }
 
-// slice a Block to one NDNLP packet
+// slice a Block to one fragment
 BOOST_AUTO_TEST_CASE(Slice1)
 {
   uint8_t blockValue[60];
@@ -98,7 +98,7 @@
                                 block.begin(),                block.end());
 }
 
-// slice a Block to four NDNLP packets
+// slice a Block to four fragments
 BOOST_AUTO_TEST_CASE(Slice4)
 {
   uint8_t blockValue[5050];
@@ -151,14 +151,15 @@
   BOOST_CHECK_EQUAL(totalPayloadSize, block.size());
 }
 
-class ReassembleFixture : protected BaseFixture
+class ReassembleFixture : protected UnitTestTimeFixture
 {
 protected:
   ReassembleFixture()
-    : m_slicer(1500)
+    : slicer(1500)
+    , pms(time::milliseconds(100))
   {
-    m_partialMessageStore.onReceive.connect([this] (const Block& block) {
-      m_received.push_back(block);
+    pms.onReceive.connect([this] (const Block& block) {
+      received.push_back(block);
     });
   }
 
@@ -171,59 +172,118 @@
   }
 
 protected:
-  ndnlp::Slicer m_slicer;
-  ndnlp::PartialMessageStore m_partialMessageStore;
+  ndnlp::Slicer slicer;
+  ndnlp::PartialMessageStore pms;
+
+  static const time::nanoseconds IDLE_DURATION;
 
   // received network layer packets
-  std::vector<Block> m_received;
+  std::vector<Block> received;
 };
 
-// reassemble one NDNLP packets into one Block
+// reassemble one fragment into one Block
 BOOST_FIXTURE_TEST_CASE(Reassemble1, ReassembleFixture)
 {
   Block block = makeBlock(60);
-  ndnlp::PacketArray pa = m_slicer.slice(block);
+  ndnlp::PacketArray pa = slicer.slice(block);
   BOOST_REQUIRE_EQUAL(pa->size(), 1);
 
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa->at(0));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  pms.receiveNdnlpData(pa->at(0));
 
-  BOOST_REQUIRE_EQUAL(m_received.size(), 1);
-  BOOST_CHECK_EQUAL_COLLECTIONS(m_received.at(0).begin(), m_received.at(0).end(),
-                                block.begin(),            block.end());
+  BOOST_REQUIRE_EQUAL(received.size(), 1);
+  BOOST_CHECK_EQUAL_COLLECTIONS(received.at(0).begin(), received.at(0).end(),
+                                block.begin(),          block.end());
 }
 
-// reassemble four and two NDNLP packets into two Blocks
+// reassemble four and two fragments into two Blocks
 BOOST_FIXTURE_TEST_CASE(Reassemble4and2, ReassembleFixture)
 {
   Block block = makeBlock(5050);
-  ndnlp::PacketArray pa = m_slicer.slice(block);
+  ndnlp::PacketArray pa = slicer.slice(block);
   BOOST_REQUIRE_EQUAL(pa->size(), 4);
 
   Block block2 = makeBlock(2000);
-  ndnlp::PacketArray pa2 = m_slicer.slice(block2);
+  ndnlp::PacketArray pa2 = slicer.slice(block2);
   BOOST_REQUIRE_EQUAL(pa2->size(), 2);
 
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa->at(0));
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa->at(1));
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa2->at(1));
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa->at(1));
-  BOOST_CHECK_EQUAL(m_received.size(), 0);
-  m_partialMessageStore.receiveNdnlpData(pa2->at(0));
-  BOOST_CHECK_EQUAL(m_received.size(), 1);
-  m_partialMessageStore.receiveNdnlpData(pa->at(3));
-  BOOST_CHECK_EQUAL(m_received.size(), 1);
-  m_partialMessageStore.receiveNdnlpData(pa->at(2));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  pms.receiveNdnlpData(pa->at(0));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
 
-  BOOST_REQUIRE_EQUAL(m_received.size(), 2);
-  BOOST_CHECK_EQUAL_COLLECTIONS(m_received.at(1).begin(), m_received.at(1).end(),
-                                block.begin(),            block.end());
-  BOOST_CHECK_EQUAL_COLLECTIONS(m_received.at(0).begin(), m_received.at(0).end(),
-                                block2.begin(),           block2.end());
+  pms.receiveNdnlpData(pa->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa2->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa2->at(0));
+  BOOST_CHECK_EQUAL(received.size(), 1);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(3));
+  BOOST_CHECK_EQUAL(received.size(), 1);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(2));
+
+  BOOST_REQUIRE_EQUAL(received.size(), 2);
+  BOOST_CHECK_EQUAL_COLLECTIONS(received.at(1).begin(), received.at(1).end(),
+                                block.begin(),          block.end());
+  BOOST_CHECK_EQUAL_COLLECTIONS(received.at(0).begin(), received.at(0).end(),
+                                block2.begin(),         block2.end());
+}
+
+// reassemble four fragments into one Block, but another two fragments are expired
+BOOST_FIXTURE_TEST_CASE(ReassembleTimeout, ReassembleFixture)
+{
+  Block block = makeBlock(5050);
+  ndnlp::PacketArray pa = slicer.slice(block);
+  BOOST_REQUIRE_EQUAL(pa->size(), 4);
+
+  Block block2 = makeBlock(2000);
+  ndnlp::PacketArray pa2 = slicer.slice(block2);
+  BOOST_REQUIRE_EQUAL(pa2->size(), 2);
+
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  pms.receiveNdnlpData(pa->at(0));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa2->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(1));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(3));
+  BOOST_CHECK_EQUAL(received.size(), 0);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa->at(2));
+  BOOST_CHECK_EQUAL(received.size(), 1);
+  this->advanceClocks(time::milliseconds(40));
+
+  pms.receiveNdnlpData(pa2->at(0)); // last fragment was received 160ms ago, expired
+  BOOST_CHECK_EQUAL(received.size(), 1);
+  this->advanceClocks(time::milliseconds(40));
+
+  BOOST_REQUIRE_EQUAL(received.size(), 1);
+  BOOST_CHECK_EQUAL_COLLECTIONS(received.at(0).begin(), received.at(0).end(),
+                                block.begin(),          block.end());
 }
 
 BOOST_AUTO_TEST_SUITE_END()