From 6564f6c164244bec8641e27d0cee85182629ab08 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 24 Oct 2014 13:43:22 -0700 Subject: [PATCH] Fix incorrect socket closure in Overlay peers: On Application exit, Overlay was calling PeerImp::close for each peer. The implementation of PeerImp::close only canceled all pending I/O and did not call functions necessary for proper transition of Peer state during socket closure. The correct transition is ensured by calling PeerImp::detach. This changes PeerImp::close to call PeerImp::detach instead, ensuring that Overlay invariants are maintained. Specifically, that reference counts for pending I/O on peers will be correctly unwound by canceling operations and that the Peer object will be destroyed, thus allowing the Overlay to stop correctly. --- src/ripple/overlay/impl/OverlayImpl.h | 75 +++++++++++++-------------- src/ripple/overlay/impl/PeerImp.cpp | 8 +-- 2 files changed, 38 insertions(+), 45 deletions(-) diff --git a/src/ripple/overlay/impl/OverlayImpl.h b/src/ripple/overlay/impl/OverlayImpl.h index 9f40aaf36e..8b7b64899c 100644 --- a/src/ripple/overlay/impl/OverlayImpl.h +++ b/src/ripple/overlay/impl/OverlayImpl.h @@ -111,6 +111,42 @@ public: ~OverlayImpl (); + PeerSequence + getActivePeers () override; + + Peer::ptr + findPeerByShortID (Peer::ShortId const& id) override; + + /** Process an incoming connection using the Peer protocol. + The caller transfers ownership of the socket via rvalue move. + @param socket A socket in the accepted state. + */ + void + accept (socket_type&& socket); + + Peer::ShortId + next_id(); + + void + remove (PeerFinder::Slot::ptr const& slot); + + /** Called when a peer has connected successfully + This is called after the peer handshake has been completed and during + peer activation. At this point, the peer address and the public key + are known. + */ + void + activate (Peer::ptr const& peer); + + /** A peer is being disconnected + This is called during the disconnection of a known, activated peer. It + will not be called for outbound peer connections that don't succeed or + for connections of peers that are dropped prior to being activated. + */ + void + onPeerDisconnect (Peer::ptr const& peer); + +private: OverlayImpl (OverlayImpl const&) = delete; OverlayImpl& operator= (OverlayImpl const&) = delete; @@ -123,34 +159,11 @@ public: Json::Value json() override; - PeerSequence - getActivePeers () override; - - Peer::ptr - findPeerByShortID (Peer::ShortId const& id) override; - -public: - /** Process an incoming connection using the Peer protocol. - The caller transfers ownership of the socket via rvalue move. - @param socket A socket in the accepted state. - */ - void - accept (socket_type&& socket); - - Peer::ShortId - next_id(); - //-------------------------------------------------------------------------- void check_stopped (); - void - release (); - - void - remove (PeerFinder::Slot::ptr const& slot); - // // Stoppable // @@ -184,23 +197,9 @@ public: //-------------------------------------------------------------------------- - /** Called when a peer has connected successfully - This is called after the peer handshake has been completed and during - peer activation. At this point, the peer address and the public key - are known. - */ void - activate (Peer::ptr const& peer); + release(); - /** A peer is being disconnected - This is called during the disconnection of a known, activated peer. It - will not be called for outbound peer connections that don't succeed or - for connections of peers that are dropped prior to being activated. - */ - void - onPeerDisconnect (Peer::ptr const& peer); - -private: void sendpeers(); diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 10b9009e90..eb1602a6ed 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -83,13 +83,7 @@ PeerImp::start () void PeerImp::close() { - if (! strand_.running_in_this_thread()) - return strand_.post (std::bind ( - &PeerImp::close, shared_from_this())); - - error_code ec; - timer_.cancel(ec); - stream_.next_layer().close(ec); + detach("close", false); } //------------------------------------------------------------------------------