fw: invoke FaceTable::remove upon Face::onFail
refs #1358
Change-Id: Ic19a88f0aea5adefbd3179de09f9f4e049922ff8
diff --git a/daemon/fw/face-table.cpp b/daemon/fw/face-table.cpp
index a3013f6..1ed79ad 100644
--- a/daemon/fw/face-table.cpp
+++ b/daemon/fw/face-table.cpp
@@ -31,9 +31,11 @@
NFD_LOG_INFO("addFace id=" << faceId);
face->onReceiveInterest += bind(&Forwarder::onInterest,
- &m_forwarder, boost::ref(*face), _1);
+ &m_forwarder, boost::ref(*face), _1);
face->onReceiveData += bind(&Forwarder::onData,
- &m_forwarder, boost::ref(*face), _1);
+ &m_forwarder, boost::ref(*face), _1);
+ face->onFail += bind(&FaceTable::remove,
+ this, face);
}
void
@@ -48,6 +50,7 @@
// does not support only removing Forwarder's subscription
face->onReceiveInterest.clear();
face->onReceiveData .clear();
+ // don't clear onFail because other functions may need to execute
m_forwarder.getFib().removeNextHopFromAllEntries(face);
}
diff --git a/daemon/fw/face-table.hpp b/daemon/fw/face-table.hpp
index 2415568..2af2b6b 100644
--- a/daemon/fw/face-table.hpp
+++ b/daemon/fw/face-table.hpp
@@ -29,9 +29,6 @@
VIRTUAL_WITH_TESTS void
add(shared_ptr<Face> face);
- VIRTUAL_WITH_TESTS void
- remove(shared_ptr<Face> face);
-
VIRTUAL_WITH_TESTS shared_ptr<Face>
get(FaceId id) const;
@@ -52,6 +49,12 @@
end() const;
private:
+ // remove is private because it's a subscriber of face.onFail event.
+ // face->close() closes a face and would trigger .remove(face)
+ void
+ remove(shared_ptr<Face> face);
+
+private:
Forwarder& m_forwarder;
FaceId m_lastFaceId;
FaceMap m_faces;
diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp
index 2d4d325..62612f2 100644
--- a/daemon/fw/forwarder.hpp
+++ b/daemon/fw/forwarder.hpp
@@ -53,13 +53,6 @@
void
addFace(shared_ptr<Face> face);
- /** \brief remove existing Face
- *
- * shortcut to .getFaceTable().remove(face)
- */
- void
- removeFace(shared_ptr<Face> face);
-
public: // forwarding entrypoints and tables
void
onInterest(Face& face, const Interest& interest);
@@ -188,12 +181,6 @@
}
inline void
-Forwarder::removeFace(shared_ptr<Face> face)
-{
- m_faceTable.remove(face);
-}
-
-inline void
Forwarder::onInterest(Face& face, const Interest& interest)
{
this->onIncomingInterest(face, interest);
diff --git a/daemon/mgmt/face-manager.cpp b/daemon/mgmt/face-manager.cpp
index bf31a10..6407237 100644
--- a/daemon/mgmt/face-manager.cpp
+++ b/daemon/mgmt/face-manager.cpp
@@ -632,7 +632,7 @@
shared_ptr<Face> target = m_faceTable.get(options.getFaceId());
if (target)
{
- m_faceTable.remove(target);
+ // don't call m_faceTable.remove(target): it's called by target->close() via onFail
target->close();
}
sendResponse(requestName, 200, "Success");
diff --git a/tests/face/dummy-face.hpp b/tests/face/dummy-face.hpp
index a28a1bb..4e4bb57 100644
--- a/tests/face/dummy-face.hpp
+++ b/tests/face/dummy-face.hpp
@@ -44,6 +44,7 @@
virtual void
close()
{
+ this->onFail("close");
}
void
diff --git a/tests/fw/client-control-strategy.cpp b/tests/fw/client-control-strategy.cpp
index aab5baa..51534a4 100644
--- a/tests/fw/client-control-strategy.cpp
+++ b/tests/fw/client-control-strategy.cpp
@@ -64,7 +64,7 @@
shared_ptr<pit::Entry> pitEntry3 = pit.insert(*interest3).first;
pitEntry3->insertOrUpdateInRecord(face4, *interest3);
- forwarder.removeFace(face3); // face3 is removed and its FaceId becomes invalid
+ face3->close(); // face3 is closed and its FaceId becomes invalid
strategy.m_sendInterestHistory.clear();
strategy.m_rejectPendingInterestHistory.clear();
strategy.afterReceiveInterest(*face4, *interest3, fibEntry, pitEntry3);
diff --git a/tests/fw/face-table.cpp b/tests/fw/face-table.cpp
index 474a756..b07995f 100644
--- a/tests/fw/face-table.cpp
+++ b/tests/fw/face-table.cpp
@@ -32,8 +32,8 @@
BOOST_CHECK_NE(face2->getId(), INVALID_FACEID);
BOOST_CHECK_NE(face1->getId(), face2->getId());
- forwarder.removeFace(face1);
- forwarder.removeFace(face2);
+ face1->close();
+ face2->close();
BOOST_CHECK_EQUAL(face1->getId(), INVALID_FACEID);
BOOST_CHECK_EQUAL(face2->getId(), INVALID_FACEID);
@@ -77,7 +77,7 @@
BOOST_CHECK(hasFace1);
BOOST_CHECK(hasFace2);
- faceTable.remove(face1);
+ face1->close();
BOOST_CHECK_EQUAL(faceTable.size(), 1);
BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
hasFace1 = hasFace2 = false;
diff --git a/tests/mgmt/face-manager.cpp b/tests/mgmt/face-manager.cpp
index 80e14d7..310a425 100644
--- a/tests/mgmt/face-manager.cpp
+++ b/tests/mgmt/face-manager.cpp
@@ -81,11 +81,11 @@
m_addFired = true;
}
- virtual void
- remove(shared_ptr<Face> face)
- {
- m_removeFired = true;
- }
+// virtual void
+// remove(shared_ptr<Face> face)
+// {
+// m_removeFired = true;
+// }
virtual shared_ptr<Face>
get(FaceId id) const
@@ -100,11 +100,11 @@
return m_addFired;
}
- bool
- didRemoveFire() const
- {
- return m_removeFired;
- }
+// bool
+// didRemoveFire() const
+// {
+// return m_removeFired;
+// }
bool
didGetFire() const
@@ -292,11 +292,11 @@
return m_faceTable.didAddFire();
}
- bool
- didFaceTableRemoveFire() const
- {
- return m_faceTable.didRemoveFire();
- }
+// bool
+// didFaceTableRemoveFire() const
+// {
+// return m_faceTable.didRemoveFire();
+// }
bool
didFaceTableGetFire() const
@@ -1022,7 +1022,7 @@
destroyFace(command->getName(), options);
BOOST_REQUIRE(didCallbackFire());
- BOOST_CHECK(TestFaceTableFixture::m_faceTable.didRemoveFire());
+// BOOST_CHECK(TestFaceTableFixture::m_faceTable.didRemoveFire());
BOOST_CHECK(TestFaceTableFixture::m_faceTable.getDummyFace()->didCloseFire());
}