diff --git a/src/peer.cpp b/src/peer.cpp index 7cb221793d3b1a1f42594f590b46448717c166db..f6f29e7f1df936c97285d80685a63a4019c7bc68 100644 --- a/src/peer.cpp +++ b/src/peer.cpp @@ -568,9 +568,11 @@ Peer::~Peer() { } // Prevent deletion if there are any jobs remaining - if (job_count_ > 0 && ftl::pool.size() > 0) { + int count = 10; + while (job_count_ > 0 && ftl::pool.size() > 0 && count-- > 0) { DLOG(1) << "Waiting on peer jobs... " << job_count_; std::this_thread::sleep_for(std::chrono::milliseconds(2)); - if (job_count_ > 0) LOG(FATAL) << "Peer jobs not terminated"; } + + if (job_count_ > 0) LOG(FATAL) << "Peer jobs not terminated"; } diff --git a/src/universe.cpp b/src/universe.cpp index 4be8d39c97598135fc4d9a3e044f1ad558eb75be..b770b4611d7eb0707091373292cb3c41ab604d0f 100644 --- a/src/universe.cpp +++ b/src/universe.cpp @@ -192,11 +192,17 @@ void Universe::shutdown() { active_ = false; thread_.join(); - // FIXME: This shouldn't be needed - if (peer_instances_ > 0 && ftl::pool.size() > 0) { - DLOG(WARNING) << "Waiting on peer destruction... " << peer_instances_; + _cleanupPeers(); + while (garbage_.size() > 0) { + _garbage(); + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + + // Try 10 times to delete the peers + // Note: other threads may still be using the peer object + int count = 10; + while (peer_instances_ > 0 && count-- > 0) { std::this_thread::sleep_for(std::chrono::milliseconds(2)); - if (peer_instances_ > 0) LOG(FATAL) << "Peers not destroyed"; } } @@ -467,14 +473,18 @@ void Universe::_periodic() { // Garbage peers may not be needed any more if (garbage_.size() > 0) { - UNIQUE_LOCK(net_mutex_, lk); - // Only do garbage if processing is idle. - if (ftl::pool.n_idle() == ftl::pool.size()) { - if (garbage_.size() > 0) DLOG(1) << "Garbage collection"; - while (garbage_.size() > 0) { - garbage_.front().reset(); - garbage_.pop_front(); - } + _garbage(); + } +} + +void Universe::_garbage() { + UNIQUE_LOCK(net_mutex_, lk); + // Only do garbage if processing is idle. + if (ftl::pool.n_idle() == ftl::pool.size()) { + if (garbage_.size() > 0) DLOG(1) << "Garbage collection"; + while (garbage_.size() > 0) { + garbage_.front().reset(); + garbage_.pop_front(); } } } diff --git a/src/universe.hpp b/src/universe.hpp index 4a8911e1421beb5692e36251767fc2d930551d6e..5ad13c3af2c421518c87ae892e2935b53a527e1e 100644 --- a/src/universe.hpp +++ b/src/universe.hpp @@ -191,6 +191,7 @@ class Universe { void _notifyDisconnect(ftl::net::Peer *); void _notifyError(ftl::net::Peer *, ftl::protocol::Error, const std::string &); void _periodic(); + void _garbage(); ftl::net::PeerPtr _findPeer(const ftl::net::Peer *p); void _removePeer(PeerPtr &p); void _insertPeer(const ftl::net::PeerPtr &ptr); diff --git a/test/net_integration.cpp b/test/net_integration.cpp index b1490f721326d5c6b7560e3bdcbcb23115b496eb..c1a04153071262289df0291f64598174938f7463 100644 --- a/test/net_integration.cpp +++ b/test/net_integration.cpp @@ -24,6 +24,25 @@ static bool try_for(int count, const std::function<bool()> &f) { // --- Tests ------------------------------------------------------------------- +TEST_CASE("Garbage bug", "[net]") { + auto self = ftl::createDummySelf(); + + self->listen(ftl::URI("tcp://localhost:0")); + + auto uri = "tcp://127.0.0.1:" + std::to_string(self->getListeningURIs().front().getPort()); + LOG(INFO) << uri; + auto p = ftl::connectNode(uri); + REQUIRE( p ); + + REQUIRE( p->waitConnection(5) ); + + REQUIRE( self->waitConnections(5) == 1 ); + REQUIRE( ftl::getSelf()->numberOfNodes() == 1); + + p.reset(); + ftl::protocol::reset(); +} + TEST_CASE("Listen and Connect", "[net]") { auto self = ftl::createDummySelf();