From 1a56b9c5f2f87e95b16cdc155693e6e634a1111d Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 7 Jun 2017 13:18:32 -0700 Subject: [PATCH] Replace DeadlineTimer with asio::steadyTimer (RIPD-1356): The two active users of DeadlineTimer, NetworkOPs and Application, now use asio::steady_timers rather than DeadlineTimer. DeadlineTimer is removed since it is no longer used. To assure that all in-flight closures on timers are done before Stoppables call stopped(), the JobCounter is made more generic. It's now a ClosureCounter. The ClosureCounter is currently used to count closures in flight for the JobQueue, NetworkOPs, and the Application. --- Builds/VisualStudio2015/RippleD.vcxproj | 24 +- .../VisualStudio2015/RippleD.vcxproj.filters | 21 +- src/ripple/app/main/Application.cpp | 106 ++++-- src/ripple/app/misc/NetworkOPs.cpp | 128 +++++-- src/ripple/app/misc/NetworkOPs.h | 9 +- src/ripple/core/ClosureCounter.h | 207 +++++++++++ src/ripple/core/DeadlineTimer.h | 119 ------ src/ripple/core/JobCounter.h | 205 ----------- src/ripple/core/JobQueue.h | 1 - src/ripple/core/Stoppable.h | 6 +- src/ripple/core/impl/DeadlineTimer.cpp | 338 ------------------ src/ripple/unity/core.cpp | 1 - src/test/core/ClosureCounter_test.cpp | 329 +++++++++++++++++ src/test/core/DeadlineTimer_test.cpp | 123 ------- src/test/core/JobCounter_test.cpp | 131 ------- src/test/server/ServerStatus_test.cpp | 1 - src/test/unity/core_test_unity.cpp | 3 +- 17 files changed, 733 insertions(+), 1019 deletions(-) create mode 100644 src/ripple/core/ClosureCounter.h delete mode 100644 src/ripple/core/DeadlineTimer.h delete mode 100644 src/ripple/core/JobCounter.h delete mode 100644 src/ripple/core/impl/DeadlineTimer.cpp create mode 100644 src/test/core/ClosureCounter_test.cpp delete mode 100644 src/test/core/DeadlineTimer_test.cpp delete mode 100644 src/test/core/JobCounter_test.cpp diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj index 629ce32ba2..d0f2b8e984 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj +++ b/Builds/VisualStudio2015/RippleD.vcxproj @@ -1879,6 +1879,8 @@ + + @@ -1887,8 +1889,6 @@ - - True True @@ -1901,12 +1901,6 @@ ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) - - True - True - ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) - ..\..\src\soci\src\core;..\..\src\sqlite;%(AdditionalIncludeDirectories) - True True @@ -1981,8 +1975,6 @@ - - @@ -4491,6 +4483,10 @@ True True + + True + True + True True @@ -4503,14 +4499,6 @@ True True - - True - True - - - True - True - True True diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters index 244c56165d..f219baaacc 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters @@ -2529,6 +2529,9 @@ ripple\consensus + + ripple\core + ripple\core @@ -2541,18 +2544,12 @@ ripple\core - - ripple\core - ripple\core\impl ripple\core\impl - - ripple\core\impl - ripple\core\impl @@ -2598,9 +2595,6 @@ ripple\core - - ripple\core - ripple\core @@ -5250,6 +5244,9 @@ test\consensus + + test\core + test\core @@ -5259,12 +5256,6 @@ test\core - - test\core - - - test\core - test\core diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index d952b1736d..2b472513ac 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -48,7 +48,6 @@ #include #include #include -#include #include #include #include @@ -56,6 +55,7 @@ #include #include #include +#include #include namespace ripple { @@ -218,7 +218,6 @@ supportedAmendments (); class ApplicationImp : public Application , public RootStoppable - , public DeadlineTimer::Listener , public BasicApp { private: @@ -334,8 +333,9 @@ public: RCLValidations mValidations; std::unique_ptr m_loadManager; std::unique_ptr txQ_; - DeadlineTimer m_sweepTimer; - DeadlineTimer m_entropyTimer; + ClosureCounter waitHandlerCounter_; + boost::asio::steady_timer sweepTimer_; + boost::asio::steady_timer entropyTimer_; bool startTimers_; std::unique_ptr mTxnDB; @@ -448,7 +448,7 @@ public: , m_networkOPs (make_NetworkOPs (*this, stopwatch(), config_->standalone(), config_->NETWORK_QUORUM, config_->START_VALID, *m_jobQueue, *m_ledgerMaster, *m_jobQueue, validatorKeys_, - logs_->journal("NetworkOPs"))) + get_io_service(), logs_->journal("NetworkOPs"))) , cluster_ (std::make_unique ( logs_->journal("Overlay"))) @@ -481,9 +481,9 @@ public: , txQ_(make_TxQ(setup_TxQ(*config_), logs_->journal("TxQ"))) - , m_sweepTimer (this) + , sweepTimer_ (get_io_service()) - , m_entropyTimer (this) + , entropyTimer_ (get_io_service()) , startTimers_ (false) @@ -827,8 +827,8 @@ public: using namespace std::chrono_literals; if(startTimers_) { - m_sweepTimer.setExpiration (10s); - m_entropyTimer.setRecurringExpiration (5min); + setSweepTimer(); + setEntropyTimer(); } m_io_latency_sampler.start(); @@ -858,11 +858,28 @@ public: // things will happen. m_resolver->stop (); - if(startTimers_) { - m_sweepTimer.cancel (); - m_entropyTimer.cancel (); + boost::system::error_code ec; + sweepTimer_.cancel (ec); + if (ec) + { + JLOG (m_journal.error()) + << "Application: sweepTimer cancel error: " + << ec.message(); + } + + ec.clear(); + entropyTimer_.cancel (ec); + if (ec) + { + JLOG (m_journal.error()) + << "Application: entropyTimer cancel error: " + << ec.message(); + } } + // Make sure that any waitHandlers pending in our timers are done + // before we declare ourselves stopped. + waitHandlerCounter_.join("Application", 1s, m_journal); mValidations.flush (); @@ -884,7 +901,7 @@ public: stopped (); } - //------------------------------------------------------------------------------ + //-------------------------------------------------------------------------- // // PropertyStream // @@ -893,20 +910,64 @@ public: { } - //------------------------------------------------------------------------------ + //-------------------------------------------------------------------------- - void onDeadlineTimer (DeadlineTimer& timer) override + void setSweepTimer () { - if (timer == m_entropyTimer) + // Only start the timer if waitHandlerCounter_ is not yet joined. + if (auto optionalCountedHandler = waitHandlerCounter_.wrap ( + [this] (boost::system::error_code const& e) + { + if ((e.value() == boost::system::errc::success) && + (! m_jobQueue->isStopped())) + { + m_jobQueue->addJob( + jtSWEEP, "sweep", [this] (Job&) { doSweep(); }); + } + // Recover as best we can if an unexpected error occurs. + if (e.value() != boost::system::errc::success && + e.value() != boost::asio::error::operation_aborted) + { + // Try again later and hope for the best. + JLOG (m_journal.error()) + << "Sweep timer got error '" << e.message() + << "'. Restarting timer."; + setSweepTimer(); + } + })) { - crypto_prng().mix_entropy (); - return; + sweepTimer_.expires_from_now ( + std::chrono::seconds {config_->getSize (siSweepInterval)}); + sweepTimer_.async_wait (std::move (*optionalCountedHandler)); } + } - if (timer == m_sweepTimer) + void setEntropyTimer () + { + // Only start the timer if waitHandlerCounter_ is not yet joined. + if (auto optionalCountedHandler = waitHandlerCounter_.wrap ( + [this] (boost::system::error_code const& e) + { + if (e.value() == boost::system::errc::success) + { + crypto_prng().mix_entropy(); + setEntropyTimer(); + } + // Recover as best we can if an unexpected error occurs. + if (e.value() != boost::system::errc::success && + e.value() != boost::asio::error::operation_aborted) + { + // Try again later and hope for the best. + JLOG (m_journal.error()) + << "Entropy timer got error '" << e.message() + << "'. Restarting timer."; + setEntropyTimer(); + } + })) { - m_jobQueue->addJob( - jtSWEEP, "sweep", [this] (Job&) { doSweep(); }); + using namespace std::chrono_literals; + entropyTimer_.expires_from_now (5min); + entropyTimer_.async_wait (std::move (*optionalCountedHandler)); } } @@ -942,8 +1003,7 @@ public: cachedSLEs_.expire(); // Set timer to do another sweep later. - m_sweepTimer.setExpiration ( - std::chrono::seconds {config_->getSize (siSweepInterval)}); + setSweepTimer(); } diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index e0e84c80b5..8d37ae1c60 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -58,12 +57,12 @@ #include #include #include +#include namespace ripple { class NetworkOPsImp final : public NetworkOPs - , public DeadlineTimer::Listener { /** * Transaction with input flags and results to be applied in batches. @@ -183,11 +182,11 @@ class NetworkOPsImp final public: // VFALCO TODO Make LedgerMaster a SharedPtr or a reference. // - NetworkOPsImp ( - Application& app, clock_type& clock, bool standalone, - std::size_t network_quorum, bool start_valid, JobQueue& job_queue, - LedgerMaster& ledgerMaster, Stoppable& parent, - ValidatorKeys const & validatorKeys, beast::Journal journal) + NetworkOPsImp (Application& app, NetworkOPs::clock_type& clock, + bool standalone, std::size_t network_quorum, bool start_valid, + JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, + ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, + beast::Journal journal) : NetworkOPs (parent) , app_ (app) , m_clock (clock) @@ -196,8 +195,8 @@ public: , mMode (start_valid ? omFULL : omDISCONNECTED) , mNeedNetworkLedger (false) , m_amendmentBlocked (false) - , m_heartbeatTimer (this) - , m_clusterTimer (this) + , heartbeatTimer_ (io_svc) + , clusterTimer_ (io_svc) , mConsensus (app, make_FeeVote(setup_FeeVote (app_.config().section ("voting")), app_.logs().journal("FeeVote")), @@ -481,16 +480,35 @@ public: void onStop () override { mAcquiringLedger.reset(); - m_heartbeatTimer.cancel(); - m_clusterTimer.cancel(); + { + boost::system::error_code ec; + heartbeatTimer_.cancel (ec); + if (ec) + { + JLOG (m_journal.error()) + << "NetworkOPs: heartbeatTimer cancel error: " + << ec.message(); + } + ec.clear(); + clusterTimer_.cancel (ec); + if (ec) + { + JLOG (m_journal.error()) + << "NetworkOPs: clusterTimer cancel error: " + << ec.message(); + } + } + // Make sure that any waitHandlers pending in our timers are done + // before we declare ourselves stopped. + using namespace std::chrono_literals; + waitHandlerCounter_.join("NetworkOPs", 1s, m_journal); stopped (); } private: void setHeartbeatTimer (); void setClusterTimer (); - void onDeadlineTimer (DeadlineTimer& timer) override; void processHeartbeatTimer (); void processClusterTimer (); @@ -533,8 +551,9 @@ private: std::atomic mNeedNetworkLedger; bool m_amendmentBlocked; - DeadlineTimer m_heartbeatTimer; - DeadlineTimer m_clusterTimer; + ClosureCounter waitHandlerCounter_; + boost::asio::steady_timer heartbeatTimer_; + boost::asio::steady_timer clusterTimer_; RCLConsensus mConsensus; @@ -638,28 +657,61 @@ void NetworkOPsImp::setStateTimer () void NetworkOPsImp::setHeartbeatTimer () { - m_heartbeatTimer.setExpiration (mConsensus.parms().ledgerGRANULARITY); + // Only start the timer if waitHandlerCounter_ is not yet joined. + if (auto optionalCountedHandler = waitHandlerCounter_.wrap ( + [this] (boost::system::error_code const& e) + { + if ((e.value() == boost::system::errc::success) && + (! m_job_queue.isStopped())) + { + m_job_queue.addJob (jtNETOP_TIMER, "NetOPs.heartbeat", + [this] (Job&) { processHeartbeatTimer(); }); + } + // Recover as best we can if an unexpected error occurs. + if (e.value() != boost::system::errc::success && + e.value() != boost::asio::error::operation_aborted) + { + // Try again later and hope for the best. + JLOG (m_journal.error()) + << "Heartbeat timer got error '" << e.message() + << "'. Restarting timer."; + setHeartbeatTimer(); + } + })) + { + heartbeatTimer_.expires_from_now ( + mConsensus.parms().ledgerGRANULARITY); + heartbeatTimer_.async_wait (std::move (*optionalCountedHandler)); + } } void NetworkOPsImp::setClusterTimer () { - using namespace std::chrono_literals; - m_clusterTimer.setExpiration (10s); -} - -void NetworkOPsImp::onDeadlineTimer (DeadlineTimer& timer) -{ - if (timer == m_heartbeatTimer) + // Only start the timer if waitHandlerCounter_ is not yet joined. + if (auto optionalCountedHandler = waitHandlerCounter_.wrap ( + [this] (boost::system::error_code const& e) + { + if ((e.value() == boost::system::errc::success) && + (! m_job_queue.isStopped())) + { + m_job_queue.addJob (jtNETOP_CLUSTER, "NetOPs.cluster", + [this] (Job&) { processClusterTimer(); }); + } + // Recover as best we can if an unexpected error occurs. + if (e.value() != boost::system::errc::success && + e.value() != boost::asio::error::operation_aborted) + { + // Try again later and hope for the best. + JLOG (m_journal.error()) + << "Cluster timer got error '" << e.message() + << "'. Restarting timer."; + setClusterTimer(); + } + })) { - m_job_queue.addJob ( - jtNETOP_TIMER, "NetOPs.heartbeat", - [this] (Job&) { processHeartbeatTimer(); }); - } - else if (timer == m_clusterTimer) - { - m_job_queue.addJob ( - jtNETOP_CLUSTER, "NetOPs.cluster", - [this] (Job&) { processClusterTimer(); }); + using namespace std::chrono_literals; + clusterTimer_.expires_from_now (10s); + clusterTimer_.async_wait (std::move (*optionalCountedHandler)); } } @@ -3347,13 +3399,15 @@ Json::Value NetworkOPsImp::StateAccounting::json() const //------------------------------------------------------------------------------ std::unique_ptr -make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, bool standalone, - std::size_t network_quorum, bool startvalid, - JobQueue& job_queue, LedgerMaster& ledgerMaster, - Stoppable& parent, ValidatorKeys const & validatorKeys, beast::Journal journal) +make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, + bool standalone, std::size_t network_quorum, bool startvalid, + JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, + ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, + beast::Journal journal) { - return std::make_unique (app, clock, standalone, network_quorum, - startvalid, job_queue, ledgerMaster, parent, validatorKeys, journal); + return std::make_unique (app, clock, standalone, + network_quorum, startvalid, job_queue, ledgerMaster, parent, + validatorKeys, io_svc, journal); } } // ripple diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index 555605a3c1..dd86629937 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -237,10 +237,11 @@ public: //------------------------------------------------------------------------------ std::unique_ptr -make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, bool standalone, - std::size_t network_quorum, bool start_valid, - JobQueue& job_queue, LedgerMaster& ledgerMaster, - Stoppable& parent, ValidatorKeys const & validatorKeys, beast::Journal journal); +make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, + bool standalone, std::size_t network_quorum, bool start_valid, + JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, + ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, + beast::Journal journal); } // ripple diff --git a/src/ripple/core/ClosureCounter.h b/src/ripple/core/ClosureCounter.h new file mode 100644 index 0000000000..609ffd2f92 --- /dev/null +++ b/src/ripple/core/ClosureCounter.h @@ -0,0 +1,207 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2017 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_CORE_CLOSURE_COUNTER_H_INCLUDED +#define RIPPLE_CORE_CLOSURE_COUNTER_H_INCLUDED + +#include +#include +#include +#include +#include +#include + +namespace ripple { + +// A class that does reference counting for postponed closures -- a closure +// who's execution is delayed by a timer or queue. The reference counting +// allows a Stoppable to assure that all such postponed closures are +// completed before the Stoppable declares itself stopped(). +// +// Ret_t is the type that the counted closure returns. +// Args_t are the types of the arguments that will be passed to the closure. +template +class ClosureCounter +{ +private: + std::mutex mutable mutex_ {}; + std::condition_variable allClosuresDoneCond_ {}; // guard with mutex_ + bool waitForClosures_ {false}; // guard with mutex_ + std::atomic closureCount_ {0}; + + // Increment the count. + ClosureCounter& operator++() + { + ++closureCount_; + return *this; + } + + // Decrement the count. If we're stopping and the count drops to zero + // notify allClosuresDoneCond_. + ClosureCounter& operator--() + { + // Even though closureCount_ is atomic, we decrement its value under + // a lock. This removes a small timing window that occurs if the + // waiting thread is handling a spurious wakeup when closureCount_ + // drops to zero. + std::lock_guard lock {mutex_}; + + // Update closureCount_. Notify if stopping and closureCount_ == 0. + if ((--closureCount_ == 0) && waitForClosures_) + allClosuresDoneCond_.notify_all(); + return *this; + } + + // A private template class that helps count the number of closures + // in flight. This allows Stoppables to hold off declaring stopped() + // until all their postponed closures are dispatched. + template + class Wrapper + { + private: + ClosureCounter& counter_; + std::remove_reference_t closure_; + + static_assert ( + std::is_same()...)), Ret_t>::value, + "Closure arguments don't match ClosureCounter Ret_t or Args_t"); + + public: + Wrapper() = delete; + + Wrapper (Wrapper const& rhs) + : counter_ (rhs.counter_) + , closure_ (rhs.closure_) + { + ++counter_; + } + + Wrapper (Wrapper&& rhs) + : counter_ (rhs.counter_) + , closure_ (std::move (rhs.closure_)) + { + ++counter_; + } + + Wrapper (ClosureCounter& counter, Closure&& closure) + : counter_ (counter) + , closure_ (std::forward (closure)) + { + ++counter_; + } + + Wrapper& operator=(Wrapper const& rhs) = delete; + Wrapper& operator=(Wrapper&& rhs) = delete; + + ~Wrapper() + { + --counter_; + } + + // Note that Args_t is not deduced, it is explicit. So Args_t&& + // would be an rvalue reference, not a forwarding reference. We + // want to forward exactly what the user declared. + Ret_t operator ()(Args_t... args) + { + return closure_ (std::forward(args)...); + } + }; + +public: + ClosureCounter() = default; + // Not copyable or movable. Outstanding counts would be hard to sort out. + ClosureCounter (ClosureCounter const&) = delete; + + ClosureCounter& operator=(ClosureCounter const&) = delete; + + /** Destructor verifies all in-flight closures are complete. */ + ~ClosureCounter() + { + using namespace std::chrono_literals; + join ("ClosureCounter", 1s, debugLog()); + } + + /** Returns once all counted in-flight closures are destroyed. + + @param name Name reported if join time exceeds wait. + @param wait If join() exceeds this duration report to Journal. + @param j Journal written to if wait is exceeded. + */ + void join (char const* name, + std::chrono::milliseconds wait, beast::Journal j) + { + std::unique_lock lock {mutex_}; + waitForClosures_ = true; + if (closureCount_ > 0) + { + if (! allClosuresDoneCond_.wait_for ( + lock, wait, [this] { return closureCount_ == 0; })) + { + if (auto stream = j.error()) + stream << name + << " waiting for ClosureCounter::join()."; + allClosuresDoneCond_.wait ( + lock, [this] { return closureCount_ == 0; }); + } + } + } + + /** Wrap the passed closure with a reference counter. + + @param closure Closure that accepts Args_t parameters and returns Ret_t. + @return If join() has been called returns boost::none. Otherwise + returns a boost::optional that wraps closure with a + reference counter. + */ + template + boost::optional> + wrap (Closure&& closure) + { + boost::optional> ret; + + std::lock_guard lock {mutex_}; + if (! waitForClosures_) + ret.emplace (*this, std::forward (closure)); + + return ret; + } + + /** Current number of Closures outstanding. Only useful for testing. */ + int count() const + { + return closureCount_; + } + + /** Returns true if this has been joined. + + Even if true is returned, counted closures may still be in flight. + However if (joined() && (count() == 0)) there should be no more + counted closures in flight. + */ + bool joined() const + { + std::lock_guard lock {mutex_}; + return waitForClosures_; + } +}; + +} // ripple + +#endif // RIPPLE_CORE_CLOSURE_COUNTER_H_INCLUDED diff --git a/src/ripple/core/DeadlineTimer.h b/src/ripple/core/DeadlineTimer.h deleted file mode 100644 index a2cbae5a60..0000000000 --- a/src/ripple/core/DeadlineTimer.h +++ /dev/null @@ -1,119 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#ifndef RIPPLE_CORE_DEADLINETIMER_H_INCLUDED -#define RIPPLE_CORE_DEADLINETIMER_H_INCLUDED - -#include -#include - -namespace ripple { - -/** Provides periodic or one time notifications at a specified time interval. -*/ -class DeadlineTimer - : public beast::List ::Node -{ -public: - using clock = std::chrono::steady_clock; ///< DeadlineTimer clock. - using duration = std::chrono::milliseconds; ///< DeadlineTimer duration. - /** DeadlineTimer time_point. */ - using time_point = std::chrono::time_point; - - /** Listener for a deadline timer. - - The listener is called on an auxiliary thread. It is suggested - not to perform any time consuming operations during the call. - */ - // VFALCO TODO Perhaps allow construction using a ServiceQueue to use - // for notifications. - // - class Listener - { - public: - /** Entry point called by DeadlineTimer when a deadline elapses. */ - virtual void onDeadlineTimer (DeadlineTimer&) = 0; - }; - - /** Create a deadline timer with the specified listener attached. - - @param listener pointer to Listener that is called at the deadline. - */ - explicit DeadlineTimer (Listener* listener); - - /// @cond INTERNAL - DeadlineTimer (DeadlineTimer const&) = delete; - DeadlineTimer& operator= (DeadlineTimer const&) = delete; - /// @endcond - - /** Destructor. */ - ~DeadlineTimer (); - - /** Cancel all notifications. - It is okay to call this on an inactive timer. - @note It is guaranteed that no notifications will occur after this - function returns. - */ - void cancel (); - - /** Set the timer to go off once in the future. - If the timer is already active, this will reset it. - @note If the timer is already active, the old one might go off - before this function returns. - @param delay duration until the timer will send a notification. - This must be greater than zero. - */ - void setExpiration (duration delay); - - /** Set the timer to go off repeatedly with the specified period. - If the timer is already active, this will reset it. - @note If the timer is already active, the old one might go off - before this function returns. - @param interval duration until the timer will send a notification. - This must be greater than zero. - */ - void setRecurringExpiration (duration interval); - - /** Equality comparison. - Timers are equal if they have the same address. - */ - inline bool operator== (DeadlineTimer const& other) const - { - return this == &other; - } - - /** Inequality comparison. */ - inline bool operator!= (DeadlineTimer const& other) const - { - return this != &other; - } - -private: - class Manager; - - Listener* const m_listener; - bool m_isActive; - - time_point notificationTime_; - duration recurring_; // > 0ms if recurring. -}; - -} - -#endif diff --git a/src/ripple/core/JobCounter.h b/src/ripple/core/JobCounter.h deleted file mode 100644 index a80e248242..0000000000 --- a/src/ripple/core/JobCounter.h +++ /dev/null @@ -1,205 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2017 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#ifndef RIPPLE_CORE_JOB_COUNTER_H_INCLUDED -#define RIPPLE_CORE_JOB_COUNTER_H_INCLUDED - -#include -#include -#include -#include -#include -#include -#include - -namespace ripple { - -// A class that does reference counting for Jobs. The reference counting -// allows a Stoppable to assure that all child Jobs in the JobQueue are -// completed before the Stoppable declares itself stopped(). -class JobCounter -{ -private: - std::mutex mutable mutex_ {}; - std::condition_variable allJobsDoneCond_ {}; // guard with mutex_ - bool waitForJobs_ {false}; // guard with mutex_ - std::atomic jobCount_ {0}; - - // Increment the count. - JobCounter& operator++() - { - ++jobCount_; - return *this; - } - - // Decrement the count. If we're stopping and the count drops to zero - // notify allJobsDoneCond_. - JobCounter& operator--() - { - // Even though jobCount_ is atomic, we decrement its value under a - // lock. This removes a small timing window that occurs if the - // waiting thread is handling a spurious wakeup when jobCount_ - // drops to zero. - std::lock_guard lock {mutex_}; - - // Update jobCount_. Notify if we're stopping and all jobs are done. - if ((--jobCount_ == 0) && waitForJobs_) - allJobsDoneCond_.notify_all(); - return *this; - } - - // A private template class that helps count the number of Jobs - // in flight. This allows Stoppables to hold off declaring stopped() - // until all their JobQueue Jobs are dispatched. - template - class CountedJob - { - private: - JobCounter& counter_; - JobHandler handler_; - - static_assert ( - std::is_same())), void>::value, - "JobHandler must be callable with Job&"); - - public: - CountedJob() = delete; - - CountedJob (CountedJob const& rhs) - : counter_ (rhs.counter_) - , handler_ (rhs.handler_) - { - ++counter_; - } - - CountedJob (CountedJob&& rhs) - : counter_ (rhs.counter_) - , handler_ (std::move (rhs.handler_)) - { - ++counter_; - } - - CountedJob (JobCounter& counter, JobHandler&& handler) - : counter_ (counter) - , handler_ (std::move (handler)) - { - ++counter_; - } - - CountedJob& operator=(CountedJob const& rhs) = delete; - CountedJob& operator=(CountedJob&& rhs) = delete; - - ~CountedJob() - { - --counter_; - } - - void operator ()(Job& job) - { - return handler_ (job); - } - }; - -public: - JobCounter() = default; - // Not copyable or movable. Outstanding counts would be hard to sort out. - JobCounter (JobCounter const&) = delete; - - JobCounter& operator=(JobCounter const&) = delete; - - /** Destructor verifies all in-flight jobs are complete. */ - ~JobCounter() - { - using namespace std::chrono_literals; - join ("JobCounter", 1s, debugLog()); - } - - /** Returns once all counted in-flight Jobs are destroyed. - - @param name Name reported if join time exceeds wait. - @param wait If join() exceeds this duration report to Journal. - @param j Journal written to if wait is exceeded. - */ - void join (char const* name, - std::chrono::milliseconds wait, beast::Journal j) - { - std::unique_lock lock {mutex_}; - waitForJobs_ = true; - if (jobCount_ > 0) - { - if (! allJobsDoneCond_.wait_for ( - lock, wait, [this] { return jobCount_ == 0; })) - { - if (auto stream = j.error()) - stream << name << " waiting for JobCounter::join()."; - allJobsDoneCond_.wait (lock, [this] { return jobCount_ == 0; }); - } - - } - } - - /** Wrap the passed lambda with a reference counter. - - @param handler Lambda that accepts a Job& parameter and returns void. - @return If join() has been called returns boost::none. Otherwise - returns a boost::optional that wraps handler with a - reference counter. - */ - template - boost::optional> - wrap (JobHandler&& handler) - { - // The current intention is that wrap() may only be called with an - // rvalue lambda. That can be adjusted in the future if needed, - // but the following static_assert covers current expectations. - static_assert (std::is_rvalue_reference::value, - "JobCounter::wrap() only supports rvalue lambdas."); - - boost::optional> ret; - - std::lock_guard lock {mutex_}; - if (! waitForJobs_) - ret.emplace (*this, std::move (handler)); - - return ret; - } - - /** Current number of Jobs outstanding. Only useful for testing. */ - int count() const - { - return jobCount_; - } - - /** Returns true if this has been joined. - - Even if true is returned, counted Jobs may still be in flight. - However if (joined() && (count() == 0)) there should be no more - counted Jobs in flight. - */ - bool joined() const - { - std::lock_guard lock {mutex_}; - return waitForJobs_; - } -}; - -} // ripple - -#endif // RIPPLE_CORE_JOB_COUNTER_H_INCLUDED diff --git a/src/ripple/core/JobQueue.h b/src/ripple/core/JobQueue.h index 8c46c10d24..b5c9228e89 100644 --- a/src/ripple/core/JobQueue.h +++ b/src/ripple/core/JobQueue.h @@ -22,7 +22,6 @@ #include #include -#include #include #include #include diff --git a/src/ripple/core/Stoppable.h b/src/ripple/core/Stoppable.h index aae8adef07..698e853161 100644 --- a/src/ripple/core/Stoppable.h +++ b/src/ripple/core/Stoppable.h @@ -23,7 +23,8 @@ #include #include #include -#include +#include +#include #include #include #include @@ -31,6 +32,9 @@ namespace ripple { +// Give a reasonable name for the JobCounter +using JobCounter = ClosureCounter; + class RootStoppable; /** Provides an interface for starting and stopping. diff --git a/src/ripple/core/impl/DeadlineTimer.cpp b/src/ripple/core/impl/DeadlineTimer.cpp deleted file mode 100644 index 5673fc4e9a..0000000000 --- a/src/ripple/core/impl/DeadlineTimer.cpp +++ /dev/null @@ -1,338 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include -#include -#include -#include -#include -#include - -namespace ripple { - -class DeadlineTimer::Manager -{ -private: - using Items = beast::List ; - - // Use RAII to manage our recursion counter. - // - // NOTE: Creation of any lock(mutex_) should be immediately followed - // by constructing a named CountRecursion. Otherwise the mutex recursion - // tracking will be faulty. - class CountRecursion - { - int& counter_; - - public: - CountRecursion (CountRecursion const&) = delete; - CountRecursion& operator=(CountRecursion const&) = delete; - - explicit CountRecursion (int& counter) - : counter_ {counter} - { - ++counter_; - } - - ~CountRecursion() - { - --counter_; - } - }; - - Manager () - { - thread_ = std::thread {&Manager::run, this}; - } - - ~Manager () - { - { - std::lock_guard lock {mutex_}; - CountRecursion c {recursionCount_}; - shouldExit_ = true; - wakeup_.notify_one(); - } - thread_.join(); - assert (m_items.empty ()); - } - -public: - static - Manager& - instance() - { - static Manager m; - return m; - } - - // Okay to call on an active timer. - // However, an extra notification may still happen due to concurrency. - // - void activate (DeadlineTimer& timer, - duration recurring, - time_point when) - { - using namespace std::chrono_literals; - assert (recurring >= 0ms); - - std::lock_guard lock {mutex_}; - CountRecursion c {recursionCount_}; - - if (timer.m_isActive) - { - m_items.erase (m_items.iterator_to (timer)); - - timer.m_isActive = false; - } - - timer.recurring_ = recurring; - timer.notificationTime_ = when; - - insertSorted (timer); - timer.m_isActive = true; - - wakeup_.notify_one(); - } - - // Okay to call this on an inactive timer. - // This can happen naturally based on concurrency. - // - void deactivate (DeadlineTimer& timer) - { - std::lock_guard lock {mutex_}; - CountRecursion c {recursionCount_}; - - if (timer.m_isActive) - { - m_items.erase (m_items.iterator_to (timer)); - - timer.m_isActive = false; - - wakeup_.notify_one(); - } - } - - void run () - { - using namespace std::chrono; - beast::setCurrentThreadName ("DeadlineTimer"); - bool shouldExit = true; - - do - { - { - auto const currentTime = - time_point_cast(clock::now()); - auto nextDeadline = currentTime; - - std::unique_lock lock {mutex_}; - CountRecursion c {recursionCount_}; - - // See if a timer expired - if (!shouldExit_ && !m_items.empty ()) - { - DeadlineTimer* const timer = &m_items.front (); - - // Has this timer expired? - if (timer->notificationTime_ <= currentTime) - { - // Expired, remove it from the list. - assert (timer->m_isActive); - m_items.pop_front (); - - // Is the timer recurring? - if (timer->recurring_ > 0ms) - { - // Yes so set the timer again. - timer->notificationTime_ = - currentTime + timer->recurring_; - - // Put it back into the list as active - insertSorted (*timer); - } - else - { - // Not a recurring timer, deactivate it. - timer->m_isActive = false; - } - - // Given the current code structure this call must - // happen inside the lock. Once the lock is released - // the timer might be canceled and it would be invalid - // to call timer->m_listener. - timer->m_listener->onDeadlineTimer (*timer); - - // re-loop - nextDeadline = currentTime - 1s; - } - else - { - // Timer has not yet expired. - nextDeadline = timer->notificationTime_; - - // Can't be zero and come into the else clause. - assert (nextDeadline > currentTime); - } - } - - if (!shouldExit_) - { - // It's bad news to invoke std::condition_variable_any - // wait() or wait_until() on a recursive_mutex if the - // recursion depth is greater than one. That's because - // wait() and wait_until() will only release one level - // of lock. - // - // We believe that the lock recursion depth can only be - // one at this point in the code, given the current code - // structure (December 2016). Here's why: - // - // 1. The DeadlineTimer::Manager runs exclusively on its - // own dedicated thread. This is the only thread where - // wakeup_.wait() or wakeup_.wait_until() are called. - // - // 2. So in order for the recursive_mutex to be called - // recursively, it must result from the call through - // timer->m_listener->onDeadlineTimer (*timer). - // - // 3. Any callback into DeadlineTimer from a Listener - // may do one of two things: a call to activate() or - // a call to deactivate(). Either of these will invoke - // the lock recursively. Then they both invoke - // condition_variable_any wakeup_.notify_one() under - // the recursive lock. Then they release the recursive - // lock. Once this local lock release occurs the - // recursion depth should be back to one. - // - // 4. So, once the Listener callback completes then the - // recursive_lock is no longer recursively held. That - // means when we enter the wakeup_.wait() or the - // wakeup_.wait_until() the lock is never held - // recursively. - // - // In case that analysis is, or becomes, incorrect the - // following LogicError should fire. - if (recursionCount_ != 1) - LogicError ("DeadlineTimer mutex recursion violation."); - - if (nextDeadline > currentTime) - // Wake up at the next deadline or next notify. - // Cast to clock::duration to work around VS-2015 bug. - // Harmless on other platforms. - wakeup_.wait_until (lock, - time_point_cast(nextDeadline)); - - else if (nextDeadline == currentTime) - // There is no deadline. Wake up at the next notify. - wakeup_.wait (lock); - - else; - // Do not wait. This can happen if the recurring - // timer duration is extremely short or if a listener - // burns lots of time in their callback. - } - // shouldExit is used outside the lock. - shouldExit = shouldExit_; - } // Note that we release the lock here. - - } while (!shouldExit); - } - - // Caller is responsible for locking - void insertSorted (DeadlineTimer& timer) - { - if (! m_items.empty ()) - { - Items::iterator before {m_items.begin()}; - - for (;;) - { - if (before->notificationTime_ >= timer.notificationTime_) - { - m_items.insert (before, timer); - break; - } - - ++before; - - if (before == m_items.end ()) - { - m_items.push_back (timer); - break; - } - } - } - else - { - m_items.push_back (timer); - } - } - -private: - std::recursive_mutex mutex_; - std::condition_variable_any wakeup_; // Works with std::recursive_mutex. - std::thread thread_; - bool shouldExit_ {false}; - int recursionCount_ {0}; - - Items m_items; -}; - -//------------------------------------------------------------------------------ - -DeadlineTimer::DeadlineTimer (Listener* listener) - : m_listener (listener) - , m_isActive (false) -{ -} - -DeadlineTimer::~DeadlineTimer () -{ - Manager::instance().deactivate (*this); -} - -void DeadlineTimer::cancel () -{ - Manager::instance().deactivate (*this); -} - -void DeadlineTimer::setExpiration (std::chrono::milliseconds delay) -{ - using namespace std::chrono; - assert (delay > 0ms); - - auto const when = time_point_cast(clock::now() + delay); - - Manager::instance().activate (*this, 0ms, when); -} - -void DeadlineTimer::setRecurringExpiration (std::chrono::milliseconds interval) -{ - using namespace std::chrono; - assert (interval > 0ms); - - auto const when = time_point_cast(clock::now() + interval); - - Manager::instance().activate (*this, interval, when); -} - -} // ripple diff --git a/src/ripple/unity/core.cpp b/src/ripple/unity/core.cpp index b7e5f88464..139331f8bc 100644 --- a/src/ripple/unity/core.cpp +++ b/src/ripple/unity/core.cpp @@ -21,7 +21,6 @@ #include #include -#include #include #include #include diff --git a/src/test/core/ClosureCounter_test.cpp b/src/test/core/ClosureCounter_test.cpp new file mode 100644 index 0000000000..406cac47ab --- /dev/null +++ b/src/test/core/ClosureCounter_test.cpp @@ -0,0 +1,329 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2017 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { +namespace test { + +//------------------------------------------------------------------------------ + +class ClosureCounter_test : public beast::unit_test::suite +{ + // We're only using Env for its Journal. + jtx::Env env {*this}; + beast::Journal j {env.app().journal ("ClosureCounter_test")}; + + void testConstruction() + { + // Build different kinds of ClosureCounters. + { + // Count closures that return void and take no arguments. + ClosureCounter voidCounter; + BEAST_EXPECT (voidCounter.count() == 0); + + int evidence = 0; + // Make sure voidCounter.wrap works with an rvalue closure. + auto wrapped = voidCounter.wrap ([&evidence] () { ++evidence; }); + BEAST_EXPECT (voidCounter.count() == 1); + BEAST_EXPECT (evidence == 0); + BEAST_EXPECT (wrapped); + + // wrapped() should be callable with no arguments. + (*wrapped)(); + BEAST_EXPECT (evidence == 1); + (*wrapped)(); + BEAST_EXPECT (evidence == 2); + + // Destroying the contents of wrapped should decrement voidCounter. + wrapped = boost::none; + BEAST_EXPECT (voidCounter.count() == 0); + } + { + // Count closures that return void and take one int argument. + ClosureCounter setCounter; + BEAST_EXPECT (setCounter.count() == 0); + + int evidence = 0; + // Make sure setCounter.wrap works with a non-const lvalue closure. + auto setInt = [&evidence] (int i) { evidence = i; }; + auto wrapped = setCounter.wrap (setInt); + + BEAST_EXPECT (setCounter.count() == 1); + BEAST_EXPECT (evidence == 0); + BEAST_EXPECT (wrapped); + + // wrapped() should be callable with one integer argument. + (*wrapped)(5); + BEAST_EXPECT (evidence == 5); + (*wrapped)(11); + BEAST_EXPECT (evidence == 11); + + // Destroying the contents of wrapped should decrement setCounter. + wrapped = boost::none; + BEAST_EXPECT (setCounter.count() == 0); + } + { + // Count closures that return int and take two int arguments. + ClosureCounter sumCounter; + BEAST_EXPECT (sumCounter.count() == 0); + + // Make sure sumCounter.wrap works with a const lvalue closure. + auto const sum = [] (int i, int j) { return i + j; }; + auto wrapped = sumCounter.wrap (sum); + + BEAST_EXPECT (sumCounter.count() == 1); + BEAST_EXPECT (wrapped); + + // wrapped() should be callable with two integers. + BEAST_EXPECT ((*wrapped)(5, 2) == 7); + BEAST_EXPECT ((*wrapped)(2, -8) == -6); + + // Destroying the contents of wrapped should decrement sumCounter. + wrapped = boost::none; + BEAST_EXPECT (sumCounter.count() == 0); + } + } + + // A class used to test argument passing. + class TrackedString + { + public: + int copies = {0}; + int moves = {0}; + std::string str; + + TrackedString() = delete; + + explicit TrackedString(char const* rhs) + : str (rhs) {} + + // Copy constructor + TrackedString (TrackedString const& rhs) + : copies (rhs.copies + 1) + , moves (rhs.moves) + , str (rhs.str) {} + + // Move constructor + TrackedString (TrackedString&& rhs) + : copies (rhs.copies) + , moves (rhs.moves + 1) + , str (std::move(rhs.str)) {} + + // Delete copy and move assignment. + TrackedString& operator=(TrackedString const& rhs) = delete; + + // String concatenation + TrackedString& operator+=(char const* rhs) + { + str += rhs; + return *this; + } + + friend + TrackedString operator+(TrackedString const& str, char const* rhs) + { + TrackedString ret {str}; + ret.str += rhs; + return ret; + } + }; + + void testArgs() + { + // Make sure a wrapped closure handles rvalue reference arguments + // correctly. + { + // Pass by value. + ClosureCounter strCounter; + BEAST_EXPECT (strCounter.count() == 0); + + auto wrapped = strCounter.wrap ( + [] (TrackedString in) { return in += "!"; }); + + BEAST_EXPECT (strCounter.count() == 1); + BEAST_EXPECT (wrapped); + + TrackedString const strValue ("value"); + TrackedString const result = (*wrapped)(strValue); + BEAST_EXPECT (result.copies == 2); + BEAST_EXPECT (result.moves == 1); + BEAST_EXPECT (result.str == "value!"); + BEAST_EXPECT (strValue.str.size() == 5); + } + { + // Use a const lvalue argument. + ClosureCounter strCounter; + BEAST_EXPECT (strCounter.count() == 0); + + auto wrapped = strCounter.wrap ( + [] (TrackedString const& in) { return in + "!"; }); + + BEAST_EXPECT (strCounter.count() == 1); + BEAST_EXPECT (wrapped); + + TrackedString const strConstLValue ("const lvalue"); + TrackedString const result = (*wrapped)(strConstLValue); + BEAST_EXPECT (result.copies == 1); + // BEAST_EXPECT (result.moves == ?); // moves VS == 1, gcc == 0 + BEAST_EXPECT (result.str == "const lvalue!"); + BEAST_EXPECT (strConstLValue.str.size() == 12); + } + { + // Use a non-const lvalue argument. + ClosureCounter strCounter; + BEAST_EXPECT (strCounter.count() == 0); + + auto wrapped = strCounter.wrap ( + [] (TrackedString& in) { return in += "!"; }); + + BEAST_EXPECT (strCounter.count() == 1); + BEAST_EXPECT (wrapped); + + TrackedString strLValue ("lvalue"); + TrackedString const result = (*wrapped)(strLValue); + BEAST_EXPECT (result.copies == 1); + BEAST_EXPECT (result.moves == 0); + BEAST_EXPECT (result.str == "lvalue!"); + BEAST_EXPECT (strLValue.str == result.str); + } + { + // Use an rvalue argument. + ClosureCounter strCounter; + BEAST_EXPECT (strCounter.count() == 0); + + auto wrapped = strCounter.wrap ( + [] (TrackedString&& in) { + // Note that none of the compilers noticed that in was + // leaving scope. So, without intervention, they would + // do a copy for the return (June 2017). An explicit + // std::move() was required. + return std::move(in += "!"); + }); + + BEAST_EXPECT (strCounter.count() == 1); + BEAST_EXPECT (wrapped); + + // Make the string big enough to (probably) avoid the small string + // optimization. + TrackedString strRValue ("rvalue abcdefghijklmnopqrstuvwxyz"); + TrackedString const result = (*wrapped)(std::move(strRValue)); + BEAST_EXPECT (result.copies == 0); + BEAST_EXPECT (result.moves == 1); + BEAST_EXPECT (result.str == "rvalue abcdefghijklmnopqrstuvwxyz!"); + BEAST_EXPECT (strRValue.str.size() == 0); + } + } + + void testWrap() + { + // Verify reference counting. + ClosureCounter voidCounter; + BEAST_EXPECT (voidCounter.count() == 0); + { + auto wrapped1 = voidCounter.wrap ([] () {}); + BEAST_EXPECT (voidCounter.count() == 1); + { + // Copy should increase reference count. + auto wrapped2 (wrapped1); + BEAST_EXPECT (voidCounter.count() == 2); + { + // Move should increase reference count. + auto wrapped3 (std::move(wrapped2)); + BEAST_EXPECT (voidCounter.count() == 3); + { + // An additional closure also increases count. + auto wrapped4 = voidCounter.wrap ([] () {}); + BEAST_EXPECT (voidCounter.count() == 4); + } + BEAST_EXPECT (voidCounter.count() == 3); + } + BEAST_EXPECT (voidCounter.count() == 2); + } + BEAST_EXPECT (voidCounter.count() == 1); + } + BEAST_EXPECT (voidCounter.count() == 0); + + // Join with 0 count should not stall. + using namespace std::chrono_literals; + voidCounter.join("testWrap", 1ms, j); + + // Wrapping a closure after join() should return boost::none. + BEAST_EXPECT (voidCounter.wrap ([] () {}) == boost::none); + } + + void testWaitOnJoin() + { + // Verify reference counting. + ClosureCounter voidCounter; + BEAST_EXPECT (voidCounter.count() == 0); + + auto wrapped = (voidCounter.wrap ([] () {})); + BEAST_EXPECT (voidCounter.count() == 1); + + // Calling join() now should stall, so do it on a different thread. + std::atomic threadExited {false}; + std::thread localThread ([&voidCounter, &threadExited, this] () + { + // Should stall after calling join. + using namespace std::chrono_literals; + voidCounter.join("testWaitOnJoin", 1ms, j); + threadExited.store (true); + }); + + // Wait for the thread to call voidCounter.join(). + while (! voidCounter.joined()); + + // The thread should still be active after waiting 5 milliseconds. + // This is not a guarantee that join() stalled the thread, but it + // improves confidence. + using namespace std::chrono_literals; + std::this_thread::sleep_for (5ms); + BEAST_EXPECT (threadExited == false); + + // Destroy the contents of wrapped and expect the thread to exit + // (asynchronously). + wrapped = boost::none; + BEAST_EXPECT (voidCounter.count() == 0); + + // Wait for the thread to exit. + while (threadExited == false); + localThread.join(); + } + +public: + void run() + { + testConstruction(); + testArgs(); + testWrap(); + testWaitOnJoin(); + } +}; + +BEAST_DEFINE_TESTSUITE(ClosureCounter, core, ripple); + +} // test +} // ripple diff --git a/src/test/core/DeadlineTimer_test.cpp b/src/test/core/DeadlineTimer_test.cpp deleted file mode 100644 index 05681bd6b6..0000000000 --- a/src/test/core/DeadlineTimer_test.cpp +++ /dev/null @@ -1,123 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2016 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include -#include -#include - -namespace ripple { - -//------------------------------------------------------------------------------ - -class DeadlineTimer_test : public beast::unit_test::suite -{ -public: - struct TestCallback : DeadlineTimer::Listener - { - TestCallback() = default; - - void onDeadlineTimer (DeadlineTimer&) override - { - ++count; - } - - std::atomic count {0}; - }; - - void testExpiration() - { - using clock = DeadlineTimer::clock; - - using namespace std::chrono_literals; - using namespace std::this_thread; - - TestCallback cb; - DeadlineTimer dt {&cb}; - - // There are parts of this test that are somewhat race conditional. - // The test is designed to avoid spurious failures, rather than - // fail occasionally but randomly, where ever possible. So there may - // be occasional gratuitous passes. Unfortunately, since it is a - // time-based test, there may also be occasional spurious failures - // on low-powered continuous integration platforms. - { - testcase("Expiration"); - - // Set a deadline timer that should only fire once in 5ms. - cb.count = 0; - auto const startTime = clock::now(); - dt.setExpiration (5ms); - - // Make sure the timer didn't fire immediately. - int const count = cb.count.load(); - if (clock::now() < startTime + 4ms) - { - BEAST_EXPECT (count == 0); - } - - // Wait until the timer should have fired and check that it did. - // In fact, we wait long enough that if it were to fire multiple - // times we'd see that. - sleep_until (startTime + 50ms); - BEAST_EXPECT (cb.count.load() == 1); - } - { - testcase("RecurringExpiration"); - - // Set a deadline timer that should fire once every 5ms. - cb.count = 0; - auto const startTime = clock::now(); - dt.setRecurringExpiration (5ms); - - // Make sure the timer didn't fire immediately. - { - int const count = cb.count.load(); - if (clock::now() < startTime + 4ms) - { - BEAST_EXPECT (count == 0); - } - } - - // Wait until the timer should have fired several times and - // check that it did. - sleep_until (startTime + 100ms); - { - auto const count = cb.count.load(); - BEAST_EXPECT ((count > 1) && (count < 21)); - } - - // Cancel the recurring timer and it should not fire any more. - dt.cancel(); - auto const count = cb.count.load(); - sleep_for (50ms); - BEAST_EXPECT (count == cb.count.load()); - } - } - - void run() - { - testExpiration(); - } -}; - -BEAST_DEFINE_TESTSUITE(DeadlineTimer, core, ripple); - -} \ No newline at end of file diff --git a/src/test/core/JobCounter_test.cpp b/src/test/core/JobCounter_test.cpp deleted file mode 100644 index 4cec0249ac..0000000000 --- a/src/test/core/JobCounter_test.cpp +++ /dev/null @@ -1,131 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2017 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include -#include -#include -#include -#include - -namespace ripple { -namespace test { - -//------------------------------------------------------------------------------ - -class JobCounter_test : public beast::unit_test::suite -{ - // We're only using Env for its Journal. - jtx::Env env {*this}; - beast::Journal j {env.app().journal ("JobCounter_test")}; - - void testWrap() - { - // Verify reference counting. - JobCounter jobCounter; - BEAST_EXPECT (jobCounter.count() == 0); - { - auto wrapped1 = jobCounter.wrap ([] (Job&) {}); - BEAST_EXPECT (jobCounter.count() == 1); - - // wrapped1 should be callable with a Job. - { - Job job; - (*wrapped1)(job); - } - { - // Copy should increase reference count. - auto wrapped2 (wrapped1); - BEAST_EXPECT (jobCounter.count() == 2); - { - // Move should increase reference count. - auto wrapped3 (std::move(wrapped2)); - BEAST_EXPECT (jobCounter.count() == 3); - { - // An additional Job also increases count. - auto wrapped4 = jobCounter.wrap ([] (Job&) {}); - BEAST_EXPECT (jobCounter.count() == 4); - } - BEAST_EXPECT (jobCounter.count() == 3); - } - BEAST_EXPECT (jobCounter.count() == 2); - } - BEAST_EXPECT (jobCounter.count() == 1); - } - BEAST_EXPECT (jobCounter.count() == 0); - - // Join with 0 count should not stall. - using namespace std::chrono_literals; - jobCounter.join("testWrap", 1ms, j); - - // Wrapping a Job after join() should return boost::none. - BEAST_EXPECT (jobCounter.wrap ([] (Job&) {}) == boost::none); - } - - void testWaitOnJoin() - { - // Verify reference counting. - JobCounter jobCounter; - BEAST_EXPECT (jobCounter.count() == 0); - - auto job = (jobCounter.wrap ([] (Job&) {})); - BEAST_EXPECT (jobCounter.count() == 1); - - // Calling join() now should stall, so do it on a different thread. - std::atomic threadExited {false}; - std::thread localThread ([&jobCounter, &threadExited, this] () - { - // Should stall after calling join. - using namespace std::chrono_literals; - jobCounter.join("testWaitOnJoin", 1ms, j); - threadExited.store (true); - }); - - // Wait for the thread to call jobCounter.join(). - while (! jobCounter.joined()); - - // The thread should still be active after waiting 5 milliseconds. - // This is not a guarantee that join() stalled the thread, but it - // improves confidence. - using namespace std::chrono_literals; - std::this_thread::sleep_for (5ms); - BEAST_EXPECT (threadExited == false); - - // Destroy the Job and expect the thread to exit (asynchronously). - job = boost::none; - BEAST_EXPECT (jobCounter.count() == 0); - - // Wait for the thread to exit. - while (threadExited == false); - localThread.join(); - } - -public: - void run() - { - testWrap(); - testWaitOnJoin(); - } -}; - -BEAST_DEFINE_TESTSUITE(JobCounter, core, ripple); - -} // test -} // ripple diff --git a/src/test/server/ServerStatus_test.cpp b/src/test/server/ServerStatus_test.cpp index 7a32987131..2386575573 100644 --- a/src/test/server/ServerStatus_test.cpp +++ b/src/test/server/ServerStatus_test.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/unity/core_test_unity.cpp b/src/test/unity/core_test_unity.cpp index c9803768e5..2db1329184 100644 --- a/src/test/unity/core_test_unity.cpp +++ b/src/test/unity/core_test_unity.cpp @@ -21,8 +21,7 @@ #include #include #include -#include -#include +#include #include #include #include