fw: avoid setting PIT unsatisfy timer twice
refs #1448
Change-Id: If48a90be2b554bc7c1ea4b869e1a927bad578e0a
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 1e68600..c6ad6b6 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -323,6 +323,7 @@
// TODO all InRecords are already expired; will this happen?
}
+ scheduler::cancel(pitEntry->m_unsatisfyTimer);
pitEntry->m_unsatisfyTimer = scheduler::schedule(lastExpiryFromNow,
bind(&Forwarder::onInterestUnsatisfied, this, pitEntry));
}
@@ -338,6 +339,7 @@
time::nanoseconds stragglerTime = time::milliseconds(100);
+ scheduler::cancel(pitEntry->m_stragglerTimer);
pitEntry->m_stragglerTimer = scheduler::schedule(stragglerTime,
bind(&Pit::erase, &m_pit, pitEntry));
}
diff --git a/daemon/table/name-tree-entry.cpp b/daemon/table/name-tree-entry.cpp
index 5f7737c..a3e4880 100644
--- a/daemon/table/name-tree-entry.cpp
+++ b/daemon/table/name-tree-entry.cpp
@@ -43,7 +43,9 @@
delete m_next;
}
-Entry::Entry(const Name& name) : m_hash(0), m_prefix(name)
+Entry::Entry(const Name& name)
+ : m_hash(0)
+ , m_prefix(name)
{
}
@@ -66,6 +68,10 @@
void
Entry::setFibEntry(shared_ptr<fib::Entry> fibEntry)
{
+ if (static_cast<bool>(fibEntry)) {
+ BOOST_ASSERT(!static_cast<bool>(fibEntry->m_nameTreeEntry));
+ }
+
if (static_cast<bool>(m_fibEntry)) {
m_fibEntry->m_nameTreeEntry.reset();
}
@@ -76,40 +82,41 @@
}
void
-Entry::insertPitEntry(shared_ptr<pit::Entry> pit)
+Entry::insertPitEntry(shared_ptr<pit::Entry> pitEntry)
{
- if (static_cast<bool>(pit)) {
- pit->m_nameTreeEntry = this->shared_from_this();
- m_pitEntries.push_back(pit);
- }
-}
+ BOOST_ASSERT(static_cast<bool>(pitEntry));
+ BOOST_ASSERT(!static_cast<bool>(pitEntry->m_nameTreeEntry));
-bool
-Entry::erasePitEntry(shared_ptr<pit::Entry> pit)
-{
- for (size_t i = 0; i < m_pitEntries.size(); i++) {
- if (m_pitEntries[i] == pit) {
- BOOST_ASSERT(pit->m_nameTreeEntry);
-
- pit->m_nameTreeEntry.reset();
- // copy the last item to the current position
- m_pitEntries[i] = m_pitEntries[m_pitEntries.size() - 1];
- // then erase the last item
- m_pitEntries.pop_back();
- return true; // success
- }
- }
- // not found this entry
- return false; // failure
+ m_pitEntries.push_back(pitEntry);
+ pitEntry->m_nameTreeEntry = this->shared_from_this();
}
void
-Entry::setMeasurementsEntry(shared_ptr<measurements::Entry> measurements)
+Entry::erasePitEntry(shared_ptr<pit::Entry> pitEntry)
{
+ BOOST_ASSERT(static_cast<bool>(pitEntry));
+ BOOST_ASSERT(pitEntry->m_nameTreeEntry.get() == this);
+
+ std::vector<shared_ptr<pit::Entry> >::iterator it =
+ std::find(m_pitEntries.begin(), m_pitEntries.end(), pitEntry);
+ BOOST_ASSERT(it != m_pitEntries.end());
+
+ *it = m_pitEntries.back();
+ m_pitEntries.pop_back();
+ pitEntry->m_nameTreeEntry.reset();
+}
+
+void
+Entry::setMeasurementsEntry(shared_ptr<measurements::Entry> measurementsEntry)
+{
+ if (static_cast<bool>(measurementsEntry)) {
+ BOOST_ASSERT(!static_cast<bool>(measurementsEntry->m_nameTreeEntry));
+ }
+
if (static_cast<bool>(m_measurementsEntry)) {
m_measurementsEntry->m_nameTreeEntry.reset();
}
- m_measurementsEntry = measurements;
+ m_measurementsEntry = measurementsEntry;
if (static_cast<bool>(m_measurementsEntry)) {
m_measurementsEntry->m_nameTreeEntry = this->shared_from_this();
}
@@ -118,6 +125,10 @@
void
Entry::setStrategyChoiceEntry(shared_ptr<strategy_choice::Entry> strategyChoiceEntry)
{
+ if (static_cast<bool>(strategyChoiceEntry)) {
+ BOOST_ASSERT(!static_cast<bool>(strategyChoiceEntry->m_nameTreeEntry));
+ }
+
if (static_cast<bool>(m_strategyChoiceEntry)) {
m_strategyChoiceEntry->m_nameTreeEntry.reset();
}
diff --git a/daemon/table/name-tree-entry.hpp b/daemon/table/name-tree-entry.hpp
index 6cf5bd6..0a0e05e 100644
--- a/daemon/table/name-tree-entry.hpp
+++ b/daemon/table/name-tree-entry.hpp
@@ -65,8 +65,6 @@
*/
class Entry : public enable_shared_from_this<Entry>, noncopyable
{
- // Make private members accessible by Name Tree
- friend class nfd::NameTree;
public:
explicit
Entry(const Name& prefix);
@@ -104,26 +102,19 @@
getFibEntry() const;
void
- insertPitEntry(shared_ptr<pit::Entry> pit);
+ insertPitEntry(shared_ptr<pit::Entry> pitEntry);
+
+ void
+ erasePitEntry(shared_ptr<pit::Entry> pitEntry);
bool
hasPitEntries() const;
- std::vector<shared_ptr<pit::Entry> >&
- getPitEntries();
-
const std::vector<shared_ptr<pit::Entry> >&
getPitEntries() const;
- /**
- * \brief Erase a PIT Entry
- * \details The address of this PIT Entry is required
- */
- bool
- erasePitEntry(shared_ptr<pit::Entry> pit);
-
void
- setMeasurementsEntry(shared_ptr<measurements::Entry> measurements);
+ setMeasurementsEntry(shared_ptr<measurements::Entry> measurementsEntry);
shared_ptr<measurements::Entry>
getMeasurementsEntry() const;
@@ -146,6 +137,8 @@
// get the Name Tree Node that is associated with this Name Tree Entry
Node* m_node;
+ // Make private members accessible by Name Tree
+ friend class nfd::NameTree;
};
inline const Name&
@@ -199,12 +192,6 @@
return !m_pitEntries.empty();
}
-inline std::vector<shared_ptr<pit::Entry> >&
-Entry::getPitEntries()
-{
- return m_pitEntries;
-}
-
inline const std::vector<shared_ptr<pit::Entry> >&
Entry::getPitEntries() const
{
diff --git a/daemon/table/pit.cpp b/daemon/table/pit.cpp
index afe310a..9c2554a 100644
--- a/daemon/table/pit.cpp
+++ b/daemon/table/pit.cpp
@@ -75,13 +75,14 @@
// - Alternatively, we could try to do findExactMatch() first, if not found,
// then do lookup().
shared_ptr<name_tree::Entry> nameTreeEntry = m_nameTree.lookup(interest.getName());
-
BOOST_ASSERT(static_cast<bool>(nameTreeEntry));
- std::vector<shared_ptr<pit::Entry> >& pitEntries = nameTreeEntry->getPitEntries();
+ const std::vector<shared_ptr<pit::Entry> >& pitEntries = nameTreeEntry->getPitEntries();
// then check if this Interest is already in the PIT entries
- std::vector<shared_ptr<pit::Entry> >::iterator it = std::find_if(pitEntries.begin(), pitEntries.end(), bind(&predicate_PitEntry_similar_Interest, _1, interest));
+ std::vector<shared_ptr<pit::Entry> >::const_iterator it =
+ std::find_if(pitEntries.begin(), pitEntries.end(),
+ bind(&predicate_PitEntry_similar_Interest, _1, boost::cref(interest)));
if (it != pitEntries.end())
{
@@ -108,7 +109,7 @@
m_nameTree.findAllMatches(data.getName(), &predicate_NameTreeEntry_hasPitEntry);
it != m_nameTree.end(); it++)
{
- std::vector<shared_ptr<pit::Entry> >& pitEntries = it->getPitEntries();
+ const std::vector<shared_ptr<pit::Entry> >& pitEntries = it->getPitEntries();
for (size_t i = 0; i < pitEntries.size(); i++)
{
if (pitEntries[i]->getInterest().matchesData(data))
@@ -122,21 +123,13 @@
void
Pit::erase(shared_ptr<pit::Entry> pitEntry)
{
- // first get the NPE
- // If pit-entry.hpp stores a NameTree Entry for each PIT, we could also use the get() method
- // directly, saving one hash computation.
- shared_ptr<name_tree::Entry> nameTreeEntry = m_nameTree.findExactMatch(pitEntry->getName());
-
+ shared_ptr<name_tree::Entry> nameTreeEntry = m_nameTree.get(*pitEntry);
BOOST_ASSERT(static_cast<bool>(nameTreeEntry));
- // erase this PIT entry
- if (static_cast<bool>(nameTreeEntry))
- {
- nameTreeEntry->erasePitEntry(pitEntry);
- m_nameTree.eraseEntryIfEmpty(nameTreeEntry);
+ nameTreeEntry->erasePitEntry(pitEntry);
+ m_nameTree.eraseEntryIfEmpty(nameTreeEntry);
- m_nItems--;
- }
+ --m_nItems;
}
} // namespace nfd
diff --git a/tests/table/name-tree.cpp b/tests/table/name-tree.cpp
index fcf373e..a652558 100644
--- a/tests/table/name-tree.cpp
+++ b/tests/table/name-tree.cpp
@@ -53,7 +53,7 @@
shared_ptr<fib::Entry> fib = npe->getFibEntry();
BOOST_CHECK(!static_cast<bool>(fib));
- std::vector< shared_ptr<pit::Entry> >& pitList = npe->getPitEntries();
+ const std::vector< shared_ptr<pit::Entry> >& pitList = npe->getPitEntries();
BOOST_CHECK_EQUAL(pitList.size(), 0);
// examine all the set method
@@ -82,33 +82,17 @@
shared_ptr<pit::Entry> PitEntry(make_shared<pit::Entry>(prefix));
shared_ptr<pit::Entry> PitEntry2(make_shared<pit::Entry>(parentName));
- Name prefix3("ndn:/named-data/research/abc/def");
- shared_ptr<pit::Entry> PitEntry3(make_shared<pit::Entry>(prefix3));
-
npe->insertPitEntry(PitEntry);
BOOST_CHECK_EQUAL(npe->getPitEntries().size(), 1);
npe->insertPitEntry(PitEntry2);
BOOST_CHECK_EQUAL(npe->getPitEntries().size(), 2);
- BOOST_CHECK_EQUAL(npe->
- erasePitEntry(PitEntry), true);
+ npe->erasePitEntry(PitEntry);
BOOST_CHECK_EQUAL(npe->getPitEntries().size(), 1);
- // erase a PIT Entry that does not exist
-
- BOOST_CHECK_EQUAL(npe->
- erasePitEntry(PitEntry3), false);
- BOOST_CHECK_EQUAL(npe->getPitEntries().size(), 1);
-
- BOOST_CHECK_EQUAL(npe->
- erasePitEntry(PitEntry2), true);
+ npe->erasePitEntry(PitEntry2);
BOOST_CHECK_EQUAL(npe->getPitEntries().size(), 0);
-
- // erase a PIT Entry that does not exist any more
-
- BOOST_CHECK_EQUAL(npe->
- erasePitEntry(PitEntry2), false);
}
BOOST_AUTO_TEST_CASE(NameTreeBasic)