From ba38bfad9deab876d89600bb080a6fb49d84f5be Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 9 Feb 2016 15:14:51 -0500 Subject: [PATCH] Server deadlock fixes --- src/ripple/server/impl/Door.cpp | 26 ++++++++---- src/ripple/server/impl/ServerImpl.cpp | 61 +++++++++++++++------------ src/ripple/server/impl/ServerImpl.h | 21 +++------ 3 files changed, 57 insertions(+), 51 deletions(-) diff --git a/src/ripple/server/impl/Door.cpp b/src/ripple/server/impl/Door.cpp index ed0a83d6a..79d472777 100644 --- a/src/ripple/server/impl/Door.cpp +++ b/src/ripple/server/impl/Door.cpp @@ -170,8 +170,6 @@ Door::Door (boost::asio::io_service& io_service, //port_->protocol.count("ws") > 0 || port_->protocol.count("http") > 0) { - server_.add (*this); - error_code ec; endpoint_type const local_address = endpoint_type(port.ip, port.port); @@ -221,7 +219,6 @@ Door::~Door() while (! list_.empty()) cond_.wait(lock); } - server_.remove (*this); } void @@ -240,12 +237,21 @@ Door::close() error_code ec; acceptor_.close(ec); // Close all detector, Peer objects - std::lock_guard lock(mutex_); - for(auto& _ : list_) + std::vector> v; { - auto const peer = _.second.lock(); - if (peer != nullptr) - peer->close(); + std::lock_guard lock(mutex_); + for(auto& p : list_) + { + auto const peer = p.second.lock(); + if (peer != nullptr) + { + peer->close(); + // Must destroy shared_ptr outside the + // lock otherwise deadlock from the + // managed object's destructor. + v.emplace_back(std::move(peer)); + } + } } } @@ -321,7 +327,9 @@ Door::do_accept (boost::asio::yield_context yield) std::move(socket), remote_address); } } + server_.remove(); } } -} +} // ripple + diff --git a/src/ripple/server/impl/ServerImpl.cpp b/src/ripple/server/impl/ServerImpl.cpp index b78aad43b..e8d782611 100644 --- a/src/ripple/server/impl/ServerImpl.cpp +++ b/src/ripple/server/impl/ServerImpl.cpp @@ -37,7 +37,7 @@ namespace HTTP { ServerImpl::ServerImpl (Handler& handler, boost::asio::io_service& io_service, beast::Journal journal) - : handler_ (handler) + : handler_ (&handler) , journal_ (journal) , io_service_ (io_service) , strand_ (io_service_) @@ -48,11 +48,13 @@ ServerImpl::ServerImpl (Handler& handler, ServerImpl::~ServerImpl() { + // Prevent call to handler + handler_ = nullptr; close(); { - // Block until all Door objects destroyed + // Block until all Doors are done accepting std::unique_lock lock(mutex_); - while (! list_.empty()) + while (accepting_ > 0) cond_.wait(lock); } } @@ -62,10 +64,17 @@ ServerImpl::ports (std::vector const& ports) { if (closed()) Throw ("ports() on closed HTTP::Server"); - for(auto const& _ : ports) - if (! _.websockets()) - std::make_shared( - io_service_, *this, _)->run(); + for(auto const& port : ports) + { + if (! port.websockets()) + { + ++accepting_; + list_.emplace_back(std::make_shared( + io_service_, *this, port)); + } + } + for(auto const& door : list_) + door->run(); } void @@ -109,48 +118,44 @@ ServerImpl::onWrite (beast::PropertyStream::Map& map) void ServerImpl::close() { - bool stopped = false; + Handler* h = nullptr; { std::lock_guard lock(mutex_); if (work_) { work_ = boost::none; // Close all Door objects - if (list_.empty()) - stopped = true; + if (accepting_ == 0) + { + std::swap (h, handler_); + } else - for(auto& _ :list_) - _.close(); + { + for(auto& door : list_) + door->close(); + } } } - if (stopped) - handler_.onStopped(*this); + if (h) + h->onStopped(*this); } //-------------------------------------------------------------------------- void -ServerImpl::add (Child& child) +ServerImpl::remove() { - std::lock_guard lock(mutex_); - list_.push_back(child); -} - -void -ServerImpl::remove (Child& child) -{ - bool stopped = false; + Handler* h = nullptr; { std::lock_guard lock(mutex_); - list_.erase(list_.iterator_to(child)); - if (list_.empty()) + if(--accepting_ == 0) { cond_.notify_all(); - stopped = true; + std::swap (h, handler_); } } - if (stopped) - handler_.onStopped(*this); + if (h) + h->onStopped(*this); } bool diff --git a/src/ripple/server/impl/ServerImpl.h b/src/ripple/server/impl/ServerImpl.h index fae0fb3e2..e3f12eaf6 100644 --- a/src/ripple/server/impl/ServerImpl.h +++ b/src/ripple/server/impl/ServerImpl.h @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -55,17 +54,13 @@ struct Stat class ServerImpl : public Server { public: - class Child : public boost::intrusive::list_base_hook < - boost::intrusive::link_mode > + struct Child { - public: + virtual ~Child() = default; virtual void close() = 0; }; private: - using list_type = boost::intrusive::make_list >::type; - using clock_type = std::chrono::system_clock; enum @@ -75,7 +70,7 @@ private: using Doors = std::vector >; - Handler& handler_; + Handler* handler_; beast::Journal journal_; boost::asio::io_service& io_service_; boost::asio::io_service::strand strand_; @@ -83,7 +78,8 @@ private: std::mutex mutable mutex_; std::condition_variable cond_; - list_type list_; + std::vector> list_; + std::size_t accepting_ = 0; std::deque stats_; int high_ = 0; @@ -114,7 +110,7 @@ public: Handler& handler() { - return handler_; + return *handler_; } boost::asio::io_service& @@ -124,10 +120,7 @@ public: } void - add (Child& child); - - void - remove (Child& child); + remove(); bool closed();