From b08c7d15cdf8f74b67085f0d29b5abf35e230dda Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 1 Nov 2013 13:31:38 -0700 Subject: [PATCH] Stoppable, make stop() require call to start() --- src/beast/beast/threads/Stoppable.h | 4 +++- src/beast/beast/threads/impl/Stoppable.cpp | 5 ++++- src/ripple_app/main/Application.cpp | 6 ------ src/ripple_app/main/IoServicePool.cpp | 25 ++++++++++++---------- src/ripple_app/main/IoServicePool.h | 2 ++ src/ripple_net/basics/SNTPClient.cpp | 15 +++++++------ 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/beast/beast/threads/Stoppable.h b/src/beast/beast/threads/Stoppable.h index 637b078c0..9ff21dcb5 100644 --- a/src/beast/beast/threads/Stoppable.h +++ b/src/beast/beast/threads/Stoppable.h @@ -265,6 +265,7 @@ protected: char const* m_name; RootStoppable& m_root; Child m_child; + Atomic m_started; bool volatile m_stopped; bool volatile m_childrenStopped; Children m_children; @@ -301,6 +302,8 @@ public: /** Notify a root stoppable and children to stop, and block until stopped. Has no effect if the stoppable was already notified. This blocks until the stoppable and all of its children have stopped. + Undefined behavior results if stop() is called without a previous call + to start(). Thread safety: Safe to call from any thread not associated with a Stoppable. */ @@ -316,7 +319,6 @@ public: private: Atomic m_prepared; - Atomic m_started; Atomic m_calledStop; Atomic m_calledStopAsync; }; diff --git a/src/beast/beast/threads/impl/Stoppable.cpp b/src/beast/beast/threads/impl/Stoppable.cpp index 75e01986a..35eb60df6 100644 --- a/src/beast/beast/threads/impl/Stoppable.cpp +++ b/src/beast/beast/threads/impl/Stoppable.cpp @@ -46,7 +46,7 @@ Stoppable::Stoppable (char const* name, Stoppable& parent) Stoppable::~Stoppable () { // Children must be stopped. - bassert (m_childrenStopped); + bassert (m_started.get() == 0 || m_childrenStopped); } bool Stoppable::isStopping() const @@ -177,6 +177,9 @@ void RootStoppable::start () void RootStoppable::stop (Journal journal) { + // Must have a prior call to start() + bassert (m_started.get() != 0); + if (! m_calledStop.compareAndSetBool (1, 0)) { journal.warning << "Stoppable::stop called again"; diff --git a/src/ripple_app/main/Application.cpp b/src/ripple_app/main/Application.cpp index cfa5da12b..093698b41 100644 --- a/src/ripple_app/main/Application.cpp +++ b/src/ripple_app/main/Application.cpp @@ -157,12 +157,6 @@ public: ~ApplicationImp () { - stop(); - //stop (); - - // Why is this needed here? - //m_networkOPs = nullptr; - bassert (s_instance == this); s_instance = nullptr; } diff --git a/src/ripple_app/main/IoServicePool.cpp b/src/ripple_app/main/IoServicePool.cpp index 6f04e7f63..70ac563c6 100644 --- a/src/ripple_app/main/IoServicePool.cpp +++ b/src/ripple_app/main/IoServicePool.cpp @@ -17,7 +17,6 @@ */ //============================================================================== - class IoServicePool::ServiceThread : private Thread { public: @@ -62,17 +61,9 @@ IoServicePool::IoServicePool (Stoppable& parent, String const& name, int numberO , m_name (name) , m_service (numberOfThreads) , m_work (boost::ref (m_service)) + , m_threadsDesired (numberOfThreads) { - bassert (numberOfThreads > 0); - - m_threads.ensureStorageAllocated (numberOfThreads); - - for (int i = 0; i < numberOfThreads; ++i) - { - ++m_threadsRunning; - m_threads.add (new ServiceThread (m_name, *this, m_service)); - m_threads[i]->start (); - } + bassert (m_threadsDesired > 0); } IoServicePool::~IoServicePool () @@ -90,6 +81,17 @@ IoServicePool::operator boost::asio::io_service& () return m_service; } +void IoServicePool::onStart () +{ + m_threads.ensureStorageAllocated (m_threadsDesired); + for (int i = 0; i < m_threadsDesired; ++i) + { + m_threads.add (new ServiceThread (m_name, *this, m_service)); + ++m_threadsRunning; + m_threads[i]->start (); + } +} + void IoServicePool::onStop () { // VFALCO NOTE This is a hack! We should gracefully @@ -97,6 +99,7 @@ void IoServicePool::onStop () // object using boost::optional, and let run() // just return naturally. // + //m_work = boost::none; m_service.stop (); } diff --git a/src/ripple_app/main/IoServicePool.h b/src/ripple_app/main/IoServicePool.h index ad2cdc7f9..2abb03c52 100644 --- a/src/ripple_app/main/IoServicePool.h +++ b/src/ripple_app/main/IoServicePool.h @@ -31,6 +31,7 @@ public: boost::asio::io_service& getService (); operator boost::asio::io_service& (); + void onStart (); void onStop (); void onChildrenStopped (); @@ -43,6 +44,7 @@ private: boost::asio::io_service m_service; boost::optional m_work; OwnedArray m_threads; + int m_threadsDesired; Atomic m_threadsRunning; }; diff --git a/src/ripple_net/basics/SNTPClient.cpp b/src/ripple_net/basics/SNTPClient.cpp index afbcfdc80..a44414830 100644 --- a/src/ripple_net/basics/SNTPClient.cpp +++ b/src/ripple_net/basics/SNTPClient.cpp @@ -96,8 +96,6 @@ public: mTimer.expires_from_now (boost::posix_time::seconds (NTP_QUERY_FREQUENCY)); mTimer.async_wait (boost::bind (&SNTPClientImp::timerEntry, this, boost::asio::placeholders::error)); - - startThread (); } ~SNTPClientImp () @@ -107,11 +105,9 @@ public: //-------------------------------------------------------------------------- - void run () + void onStart () { - m_io_service.run (); - - stopped (); + startThread (); } void onStop () @@ -120,6 +116,13 @@ public: m_io_service.stop (); } + void run () + { + m_io_service.run (); + + stopped (); + } + //-------------------------------------------------------------------------- void init (const std::vector& servers)