logic: Don't shutdown face when destructing Logic instance
refs: #4525
Change-Id: I3a50b818c6553d08fc807869107e53f55775c43d
diff --git a/src/logic.cpp b/src/logic.cpp
index c987109..ab0b794 100644
--- a/src/logic.cpp
+++ b/src/logic.cpp
@@ -152,9 +152,19 @@
Logic::~Logic()
{
- m_scheduler.cancelAllEvents();
+ _LOG_DEBUG_ID(">> Logic::~Logic");
+ for (const auto& pendingInterestId : m_pendingInterests) {
+ m_face.removePendingInterest(pendingInterestId);
+ }
+ if (m_outstandingInterestId != nullptr) {
+ m_face.removePendingInterest(m_outstandingInterestId);
+ m_outstandingInterestId = nullptr;
+ }
+ m_face.unsetInterestFilter(m_syncRegisteredPrefixId);
+
m_interestTable.clear();
- m_face.shutdown();
+ m_scheduler.cancelAllEvents();
+ _LOG_DEBUG_ID("<< Logic::~Logic");
}
void
@@ -321,22 +331,18 @@
void
Logic::printState(std::ostream& os) const
{
- BOOST_FOREACH(ConstLeafPtr leaf, m_state.getLeaves())
- {
- os << *leaf << "\n";
- }
+ for (const auto& leaf : m_state.getLeaves()) {
+ os << *leaf << "\n";
+ }
}
std::set<Name>
Logic::getSessionNames() const
{
std::set<Name> sessionNames;
-
- BOOST_FOREACH(ConstLeafPtr leaf, m_state.getLeaves())
- {
- sessionNames.insert(leaf->getSessionName());
- }
-
+ for (const auto& leaf : m_state.getLeaves()) {
+ sessionNames.insert(leaf->getSessionName());
+ }
return sessionNames;
}
@@ -639,11 +645,15 @@
Interest interest(m_syncReset);
interest.setMustBeFresh(true);
interest.setInterestLifetime(m_resetInterestLifetime);
- m_face.expressInterest(interest,
- bind(&Logic::onResetData, this, _1, _2),
- bind(&Logic::onSyncTimeout, this, _1), // Nack
- bind(&Logic::onSyncTimeout, this, _1));
-
+ const ndn::PendingInterestId* pendingInterestId = m_face.expressInterest(interest,
+ bind(&Logic::onResetData, this, _1, _2),
+ bind(&Logic::onSyncTimeout, this, _1), // Nack
+ bind(&Logic::onSyncTimeout, this, _1));
+ m_scheduler.scheduleEvent(m_resetInterestLifetime + ndn::time::milliseconds(5),
+ [pendingInterestId, this] {
+ cleanupPendingInterest(pendingInterestId);
+ });
+ m_pendingInterests.push_back(pendingInterestId);
_LOG_DEBUG_ID("<< Logic::sendResetInterest");
}
@@ -673,6 +683,10 @@
interest.setMustBeFresh(true);
interest.setInterestLifetime(m_syncInterestLifetime);
+ if (m_outstandingInterestId != nullptr) {
+ m_face.removePendingInterest(m_outstandingInterestId);
+ m_outstandingInterestId = nullptr;
+ }
m_outstandingInterestId = m_face.expressInterest(interest,
bind(&Logic::onSyncData, this, _1, _2),
bind(&Logic::onSyncTimeout, this, _1), // Nack
@@ -803,11 +817,15 @@
interest.setMustBeFresh(true);
interest.setInterestLifetime(m_recoveryInterestLifetime);
- m_face.expressInterest(interest,
- bind(&Logic::onRecoveryData, this, _1, _2),
- bind(&Logic::onRecoveryTimeout, this, _1), // Nack
- bind(&Logic::onRecoveryTimeout, this, _1));
-
+ const ndn::PendingInterestId* pendingInterestId = m_face.expressInterest(interest,
+ bind(&Logic::onRecoveryData, this, _1, _2),
+ bind(&Logic::onRecoveryTimeout, this, _1), // Nack
+ bind(&Logic::onRecoveryTimeout, this, _1));
+ m_scheduler.scheduleEvent(m_recoveryInterestLifetime + ndn::time::milliseconds(5),
+ [pendingInterestId, this] {
+ cleanupPendingInterest(pendingInterestId);
+ });
+ m_pendingInterests.push_back(pendingInterestId);
_LOG_DEBUG_ID("interest: " << interest.getName());
_LOG_DEBUG_ID("<< Logic::sendRecoveryInterest");
}
@@ -848,6 +866,15 @@
_LOG_DEBUG_ID("<< Logic::onRecoveryTimeout");
}
+void
+Logic::cleanupPendingInterest(const ndn::PendingInterestId* pendingInterestId)
+{
+ auto itr = std::find(m_pendingInterests.begin(), m_pendingInterests.end(), pendingInterestId);
+ if (itr != m_pendingInterests.end()) {
+ m_pendingInterests.erase(itr);
+ }
+}
+
// void
// Logic::sendExcludeInterest(const Interest& interest, const Data& data)
// {
diff --git a/src/logic.hpp b/src/logic.hpp
index a2fec3b..3e88794 100644
--- a/src/logic.hpp
+++ b/src/logic.hpp
@@ -468,6 +468,9 @@
// const State& commit,
// ConstBufferPtr previousRoot);
+ void
+ cleanupPendingInterest(const ndn::PendingInterestId* pendingInterestId);
+
public:
static const ndn::Name DEFAULT_NAME;
static const ndn::Name EMPTY_NAME;
@@ -494,6 +497,7 @@
InterestTable m_interestTable;
Name m_outstandingInterestName;
const ndn::PendingInterestId* m_outstandingInterestId;
+ std::vector<const ndn::PendingInterestId*> m_pendingInterests;
bool m_isInReset;
bool m_needPeriodReset;
diff --git a/tests/unit-tests/dummy-forwarder.cpp b/tests/unit-tests/dummy-forwarder.cpp
index 224dcd3..45a8cd6 100644
--- a/tests/unit-tests/dummy-forwarder.cpp
+++ b/tests/unit-tests/dummy-forwarder.cpp
@@ -1,6 +1,6 @@
/* -*- Mode: C++; c-file-style: "gnu"; indent-tabs-mode:nil -*- */
/*
- * Copyright (c) 2012-2017 University of California, Los Angeles
+ * Copyright (c) 2012-2018 University of California, Los Angeles
*
* This file is part of ChronoSync, synchronization library for distributed realtime
* applications for NDN.
@@ -35,29 +35,30 @@
{
auto face = std::make_shared<util::DummyClientFace>(m_io, m_keyChain,
util::DummyClientFace::Options{true, true});
- face->onSendInterest.connect([this, face] (const Interest& interest) {
+ util::DummyClientFace* self = &*face; // to prevent memory leak
+ face->onSendInterest.connect([this, self] (const Interest& interest) {
Interest i(interest);
for (auto& otherFace : m_faces) {
- if (&*face == &*otherFace) {
+ if (self == &*otherFace) {
continue;
}
m_io.post([=] { otherFace->receive(i); });
}
});
- face->onSendData.connect([this, face] (const Data& data) {
+ face->onSendData.connect([this, self] (const Data& data) {
Data d(data);
for (auto& otherFace : m_faces) {
- if (&*face == &*otherFace) {
+ if (self == &*otherFace) {
continue;
}
m_io.post([=] { otherFace->receive(d); });
}
});
- face->onSendNack.connect([this, face] (const lp::Nack& nack) {
+ face->onSendNack.connect([this, self] (const lp::Nack& nack) {
lp::Nack n(nack);
for (auto& otherFace : m_faces) {
- if (&*face == &*otherFace) {
+ if (self == &*otherFace) {
continue;
}
m_io.post([=] { otherFace->receive(n); });
@@ -68,5 +69,11 @@
return *face;
}
+void
+DummyForwarder::removeFaces()
+{
+ m_faces.clear();
+}
+
} // namespace chronosync
} // namespace ndn
diff --git a/tests/unit-tests/dummy-forwarder.hpp b/tests/unit-tests/dummy-forwarder.hpp
index 3e7d9b3..7965338 100644
--- a/tests/unit-tests/dummy-forwarder.hpp
+++ b/tests/unit-tests/dummy-forwarder.hpp
@@ -1,6 +1,6 @@
/* -*- Mode: C++; c-file-style: "gnu"; indent-tabs-mode:nil -*- */
/*
- * Copyright (c) 2012-2017 University of California, Los Angeles
+ * Copyright (c) 2012-2018 University of California, Los Angeles
*
* This file is part of ChronoSync, synchronization library for distributed realtime
* applications for NDN.
@@ -49,6 +49,9 @@
return *m_faces.at(nFace);
}
+ void
+ removeFaces();
+
private:
boost::asio::io_service& m_io;
KeyChain& m_keyChain;
diff --git a/tests/unit-tests/test-logic.cpp b/tests/unit-tests/test-logic.cpp
index 83e8708..dafc00b 100644
--- a/tests/unit-tests/test-logic.cpp
+++ b/tests/unit-tests/test-logic.cpp
@@ -114,10 +114,10 @@
BOOST_AUTO_TEST_CASE(TwoBasic)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[1]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->updateSeqNo(1);
@@ -141,13 +141,13 @@
BOOST_AUTO_TEST_CASE(ThreeBasic)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[1]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[2] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[2]);
+ handler[2] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->updateSeqNo(1);
@@ -171,10 +171,10 @@
BOOST_AUTO_TEST_CASE(ResetRecover)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[1]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->updateSeqNo(1);
@@ -188,7 +188,7 @@
BOOST_CHECK_EQUAL(handler[0]->map[handler[1]->logic.getSessionName()], 2);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[2] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[2]);
+ handler[2] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
advanceClocks(ndn::time::milliseconds(10), 100);
BOOST_CHECK_EQUAL(handler[2]->map[handler[0]->logic.getSessionName()], 1);
@@ -203,13 +203,13 @@
BOOST_AUTO_TEST_CASE(RecoverConflict)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[1]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[2] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[2]);
+ handler[2] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->updateSeqNo(1);
@@ -230,16 +230,16 @@
BOOST_AUTO_TEST_CASE(PartitionRecover)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[1]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[2] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[2]);
+ handler[2] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[3] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[3]);
+ handler[3] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[3]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->updateSeqNo(1);
@@ -291,10 +291,10 @@
BOOST_AUTO_TEST_CASE(MultipleUserUnderOneLogic)
{
- handler[0] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[0]);
+ handler[0] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
advanceClocks(ndn::time::milliseconds(10), 100);
- handler[1] = make_shared<Handler>(ref(fw.addFace()), syncPrefix, userPrefix[2]);
+ handler[1] = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
advanceClocks(ndn::time::milliseconds(10), 100);
handler[0]->logic.addUserNode(userPrefix[1]);
@@ -322,6 +322,58 @@
BOOST_CHECK_EQUAL(handler[1]->logic.getSessionNames().size(), 2);
}
+BOOST_AUTO_TEST_CASE(CancelOutstandingEvents)
+{
+ auto h1 = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[0]);
+ advanceClocks(ndn::time::milliseconds(10), 100);
+
+ auto h2 = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[1]);
+ advanceClocks(ndn::time::milliseconds(10), 100);
+
+ h1->updateSeqNo(1);
+
+ advanceClocks(ndn::time::milliseconds(10), 100);
+ BOOST_CHECK_EQUAL(h2->map[h1->logic.getSessionName()], 1);
+
+ h2->updateSeqNo(2);
+
+ advanceClocks(ndn::time::milliseconds(10), 100);
+ BOOST_CHECK_EQUAL(h1->map[h2->logic.getSessionName()], 2);
+
+ advanceClocks(ndn::time::milliseconds(10), 100);
+ auto h3 = make_shared<Handler>(fw.addFace(), syncPrefix, userPrefix[2]);
+ // Bringing this handler online later causes recovery interests to
+ // be sent -- h3 has no record of any digests
+
+ advanceClocks(ndn::time::milliseconds(10), 100);
+ BOOST_CHECK_EQUAL(h3->map[h1->logic.getSessionName()], 1);
+ BOOST_CHECK_EQUAL(h3->map[h2->logic.getSessionName()], 2);
+
+ h3->updateSeqNo(4);
+
+ advanceClocks(ndn::time::milliseconds(10), 100);
+ BOOST_CHECK_EQUAL(h2->map[h3->logic.getSessionName()], 4);
+ BOOST_CHECK_EQUAL(h1->map[h3->logic.getSessionName()], 4);
+
+ h1.reset();
+
+ BOOST_CHECK_NE(io.poll(), 0); // some cancel events handlers are expected
+ advanceClocks(ndn::time::minutes(1), 60); // should not crash
+
+ h2.reset();
+ h3.reset();
+
+ BOOST_CHECK_NE(io.poll(), 0); // some cancel events handlers are expected
+ fw.removeFaces();
+ while (io.poll() != 0); // execute all other ready events that may have been scheduled
+
+ steadyClock->advance(ndn::time::hours(1));
+ systemClock->advance(ndn::time::hours(1));
+
+ BOOST_CHECK_EQUAL(io.poll(), 0); // no delayed handlers are expected
+ BOOST_CHECK_EQUAL(io.stopped(), true); // io_service expected to be stopped
+}
+
BOOST_FIXTURE_TEST_CASE(TrimState, ndn::tests::IdentityManagementTimeFixture)
{
Name syncPrefix("/ndn/broadcast/sync");