From 4c8810d5f6b0ca8035c305f7994c02b9f18553da Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 1 Jul 2012 17:45:51 -0700 Subject: [PATCH] Fix the bug Jed reported. This was actually in the Peer code from the start. Calling a normal member function on a shared_from_this in a closure closes the pointer *immediately*, not at the time the closure runs. This can crash if the object goes away while the operation is pending. --- src/Peer.cpp | 16 ++++++++-------- src/Peer.h | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Peer.cpp b/src/Peer.cpp index e15a654dd5..eff19b3103 100644 --- a/src/Peer.cpp +++ b/src/Peer.cpp @@ -88,7 +88,7 @@ void Peer::detach(const char *rsn) mSendQ.clear(); (void) mVerifyTimer.cancel(); - mSocketSsl.async_shutdown(boost::bind(&Peer::handleShutdown, shared_from_this(), boost::asio::placeholders::error)); + mSocketSsl.async_shutdown(boost::bind(&Peer::sHandleShutdown, shared_from_this(), boost::asio::placeholders::error)); if (mNodePublic.isValid()) { @@ -165,7 +165,7 @@ void Peer::connect(const std::string strIp, int iPort) else { mVerifyTimer.expires_from_now(boost::posix_time::seconds(NODE_VERIFY_SECONDS), err); - mVerifyTimer.async_wait(boost::bind(&Peer::handleVerifyTimer, shared_from_this(), boost::asio::placeholders::error)); + mVerifyTimer.async_wait(boost::bind(&Peer::sHandleVerifyTimer, shared_from_this(), boost::asio::placeholders::error)); if (err) { @@ -183,7 +183,7 @@ void Peer::connect(const std::string strIp, int iPort) getSocket(), itrEndpoint, boost::bind( - &Peer::handleConnect, + &Peer::sHandleConnect, shared_from_this(), boost::asio::placeholders::error, boost::asio::placeholders::iterator)); @@ -223,7 +223,7 @@ void Peer::handleConnect(const boost::system::error_code& error, boost::asio::ip mSocketSsl.set_verify_mode(boost::asio::ssl::verify_none); mSocketSsl.async_handshake(boost::asio::ssl::stream::client, - boost::bind(&Peer::handleStart, shared_from_this(), boost::asio::placeholders::error)); + boost::bind(&Peer::sHandleStart, shared_from_this(), boost::asio::placeholders::error)); } } @@ -250,7 +250,7 @@ void Peer::connected(const boost::system::error_code& error) mSocketSsl.set_verify_mode(boost::asio::ssl::verify_none); mSocketSsl.async_handshake(boost::asio::ssl::stream::server, - boost::bind(&Peer::handleStart, shared_from_this(), boost::asio::placeholders::error)); + boost::bind(&Peer::sHandleStart, shared_from_this(), boost::asio::placeholders::error)); } else if (!mDetaching) { @@ -267,7 +267,7 @@ void Peer::sendPacketForce(PackedMessage::pointer packet) mSendingPacket = packet; boost::asio::async_write(mSocketSsl, boost::asio::buffer(packet->getBuffer()), - boost::bind(&Peer::handle_write, shared_from_this(), + boost::bind(&Peer::sHandle_write, shared_from_this(), boost::asio::placeholders::error, boost::asio::placeholders::bytes_transferred)); } @@ -296,7 +296,7 @@ void Peer::start_read_header() mReadbuf.resize(HEADER_SIZE); boost::asio::async_read(mSocketSsl, boost::asio::buffer(mReadbuf), - boost::bind(&Peer::handle_read_header, shared_from_this(), boost::asio::placeholders::error)); + boost::bind(&Peer::sHandle_read_header, shared_from_this(), boost::asio::placeholders::error)); } } @@ -311,7 +311,7 @@ void Peer::start_read_body(unsigned msg_len) mReadbuf.resize(HEADER_SIZE + msg_len); boost::asio::async_read(mSocketSsl, boost::asio::buffer(&mReadbuf[HEADER_SIZE], msg_len), - boost::bind(&Peer::handle_read_body, shared_from_this(), boost::asio::placeholders::error)); + boost::bind(&Peer::sHandle_read_body, shared_from_this(), boost::asio::placeholders::error)); } } diff --git a/src/Peer.h b/src/Peer.h index 3223632904..f06a8c3faa 100644 --- a/src/Peer.h +++ b/src/Peer.h @@ -24,10 +24,15 @@ typedef std::pair ipPort; class Peer : public boost::enable_shared_from_this { public: + typedef boost::shared_ptr pointer; + static const int psbGotHello = 0, psbSentHello = 1, psbInMap = 2, psbTrusted = 3; static const int psbNoLedgers = 4, psbNoTransactions = 5, psbDownLevel = 6; void handleConnect(const boost::system::error_code& error, boost::asio::ip::tcp::resolver::iterator it); + static void sHandleConnect(Peer::pointer ptr, const boost::system::error_code& error, + boost::asio::ip::tcp::resolver::iterator it) + { ptr->handleConnect(error, it); } private: bool mClientConnect; // In process of connecting as client. @@ -47,7 +52,12 @@ private: boost::asio::deadline_timer mVerifyTimer; void handleStart(const boost::system::error_code& ecResult); + static void sHandleStart(Peer::pointer ptr, const boost::system::error_code& ecResult) + { ptr->handleStart(ecResult); } + void handleVerifyTimer(const boost::system::error_code& ecResult); + static void sHandleVerifyTimer(Peer::pointer ptr, const boost::system::error_code& ecResult) + { ptr->handleVerifyTimer(ecResult); } protected: @@ -60,10 +70,21 @@ protected: Peer(boost::asio::io_service& io_service, boost::asio::ssl::context& ctx); void handleShutdown(const boost::system::error_code& error) { ; } + static void sHandleShutdown(Peer::pointer ptr, const boost::system::error_code& error) + { ptr->handleShutdown(error); } void handle_write(const boost::system::error_code& error, size_t bytes_transferred); + static void sHandle_write(Peer::pointer ptr, const boost::system::error_code& error, size_t bytes_transferred) + { ptr->handle_write(error, bytes_transferred); } + void handle_read_header(const boost::system::error_code& error); + static void sHandle_read_header(Peer::pointer ptr, const boost::system::error_code& error) + { ptr->handle_read_header(error); } + void handle_read_body(const boost::system::error_code& error); + static void sHandle_read_body(Peer::pointer ptr, const boost::system::error_code& error) + { ptr->handle_read_body(error); } + void processReadBuffer(); void start_read_header(); void start_read_body(unsigned msg_len); @@ -97,7 +118,6 @@ protected: void getSessionCookie(std::string& strDst); public: - typedef boost::shared_ptr pointer; //bool operator == (const Peer& other);