From caa4ed31de27fed7d2dd720fac6e5f5ce33bf45d Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 9 Sep 2015 12:19:54 -0700 Subject: [PATCH] Convert LoadManager to use std::thread (RIPD-236) --- src/ripple/app/main/LoadManager.cpp | 324 +++++++++++++--------------- src/ripple/app/main/LoadManager.h | 61 +++++- 2 files changed, 204 insertions(+), 181 deletions(-) diff --git a/src/ripple/app/main/LoadManager.cpp b/src/ripple/app/main/LoadManager.cpp index 08a23defe6..78bc5e649d 100644 --- a/src/ripple/app/main/LoadManager.cpp +++ b/src/ripple/app/main/LoadManager.cpp @@ -22,192 +22,176 @@ #include #include #include -#include #include #include #include -#include // -#include -#include namespace ripple { -class LoadManagerImp - : public LoadManager - , public beast::Thread +LoadManager::LoadManager ( + Application& app, Stoppable& parent, beast::Journal journal) + : Stoppable ("LoadManager", parent) + , app_ (app) + , journal_ (journal) + , deadLock_ (0) + , armed_ (false) + , stop_ (false) { -public: - using LockType = std::mutex; - using ScopedLockType = std::lock_guard ; + UptimeTimer::getInstance ().beginManualUpdates (); +} - Application& app_; - beast::Journal m_journal; - LockType mLock; - - bool mArmed; - - int mDeadLock; // Detect server deadlocks - //-------------------------------------------------------------------------- - - LoadManagerImp (Application& app, - Stoppable& parent, beast::Journal journal) - : LoadManager (parent) - , Thread ("loadmgr") - , app_ (app) - , m_journal (journal) - , mArmed (false) - , mDeadLock (0) - { - UptimeTimer::getInstance ().beginManualUpdates (); - } - - ~LoadManagerImp () +LoadManager::~LoadManager () +{ + try { UptimeTimer::getInstance ().endManualUpdates (); - - stopThread (); + onStop (); } - - //-------------------------------------------------------------------------- - // - // Stoppable - // - //-------------------------------------------------------------------------- - - void onPrepare () + catch (std::exception const& ex) { + // Swallow the exception in a destructor. + JLOG(journal_.warning) << "std::exception in ~LoadManager. " + << ex.what(); } - - void onStart () - { - m_journal.debug << "Starting"; - startThread (); - } - - void onStop () - { - if (isThreadRunning ()) - { - m_journal.debug << "Stopping"; - stopThreadAsync (); - } - else - { - stopped (); - } - } - - //-------------------------------------------------------------------------- - - void resetDeadlockDetector () - { - ScopedLockType sl (mLock); - mDeadLock = UptimeTimer::getInstance ().getElapsedSeconds (); - } - - void activateDeadlockDetector () - { - mArmed = true; - } - - void logDeadlock (int dlTime) - { - m_journal.warning << "Server stalled for " << dlTime << " seconds."; - } - - // VFALCO NOTE Where's the thread object? It's not a data member... - // - void run () - { - using clock_type = std::chrono::steady_clock; - - // Initialize the clock to the current time. - auto t = clock_type::now(); - - while (! threadShouldExit ()) - { - { - // VFALCO NOTE What is this lock protecting? - ScopedLockType sl (mLock); - - // VFALCO NOTE I think this is to reduce calls to the operating system - // for retrieving the current time. - // - // TODO Instead of incrementing can't we just retrieve the current - // time again? - // - // Manually update the timer. - UptimeTimer::getInstance ().incrementElapsedTime (); - - // Measure the amount of time we have been deadlocked, in seconds. - // - // VFALCO NOTE mDeadLock is a canary for detecting the condition. - int const timeSpentDeadlocked = UptimeTimer::getInstance ().getElapsedSeconds () - mDeadLock; - - // VFALCO NOTE I think that "armed" refers to the deadlock detector - // - int const reportingIntervalSeconds = 10; - if (mArmed && (timeSpentDeadlocked >= reportingIntervalSeconds)) - { - // Report the deadlocked condition every 10 seconds - if ((timeSpentDeadlocked % reportingIntervalSeconds) == 0) - { - logDeadlock (timeSpentDeadlocked); - } - - // If we go over 500 seconds spent deadlocked, it means that the - // deadlock resolution code has failed, which qualifies as undefined - // behavior. - // - assert (timeSpentDeadlocked < 500); - } - } - - bool change; - - // VFALCO TODO Eliminate the dependence on the Application object. - // Choices include constructing with the job queue / feetracker. - // Another option is using an observer pattern to invert the dependency. - if (app_.getJobQueue ().isOverloaded ()) - { - if (m_journal.info) - m_journal.info << app_.getJobQueue ().getJson (0); - change = app_.getFeeTrack ().raiseLocalFee (); - } - else - { - change = app_.getFeeTrack ().lowerLocalFee (); - } - - if (change) - { - // VFALCO TODO replace this with a Listener / observer and subscribe in NetworkOPs or Application - app_.getOPs ().reportFeeChange (); - } - - t += std::chrono::seconds (1); - auto const duration = t - clock_type::now(); - - if ((duration < std::chrono::seconds (0)) || (duration > std::chrono::seconds (1))) - { - m_journal.warning << "time jump"; - t = clock_type::now(); - } - else - { - std::this_thread::sleep_for (duration); - } - } - - stopped (); - } -}; +} //------------------------------------------------------------------------------ -LoadManager::LoadManager (Stoppable& parent) - : Stoppable ("LoadManager", parent) +void LoadManager::activateDeadlockDetector () { + std::lock_guard sl (mutex_); + armed_ = true; +} + +void LoadManager::resetDeadlockDetector () +{ + auto const elapsedSeconds = + UptimeTimer::getInstance ().getElapsedSeconds (); + + std::lock_guard sl (mutex_); + deadLock_ = elapsedSeconds; +} + +//------------------------------------------------------------------------------ + +void LoadManager::onPrepare () +{ +} + +void LoadManager::onStart () +{ + JLOG(journal_.debug) << "Starting"; + assert (! thread_.joinable()); + + thread_ = std::thread {&LoadManager::run, this}; +} + +void LoadManager::onStop () +{ + if (thread_.joinable()) + { + JLOG(journal_.debug) << "Stopping"; + { + std::lock_guard sl (mutex_); + stop_ = true; + } + thread_.join(); + } + stopped (); +} + +//------------------------------------------------------------------------------ + +void LoadManager::run () +{ + beast::Thread::setCurrentThreadName ("LoadManager"); + + using clock_type = std::chrono::steady_clock; + + // Initialize the clock to the current time. + auto t = clock_type::now(); + bool stop = false; + + while (! (stop || isStopping ())) + { + { + // VFALCO NOTE I think this is to reduce calls to the operating + // system for retrieving the current time. + // + // TODO Instead of incrementing can't we just retrieve the + // current time again? + // + // Manually update the timer. + UptimeTimer::getInstance ().incrementElapsedTime (); + + // Copy out shared data under a lock. Use copies outside lock. + std::unique_lock sl (mutex_); + auto const deadLock = deadLock_; + auto const armed = armed_; + stop = stop_; + sl.unlock(); + + // Measure the amount of time we have been deadlocked, in seconds. + // + // VFALCO NOTE deadLock_ is a canary for detecting the condition. + int const timeSpentDeadlocked = + UptimeTimer::getInstance ().getElapsedSeconds () - deadLock; + + // VFALCO NOTE I think that "armed" refers to the deadlock detector. + // + int const reportingIntervalSeconds = 10; + if (armed && (timeSpentDeadlocked >= reportingIntervalSeconds)) + { + // Report the deadlocked condition every 10 seconds + if ((timeSpentDeadlocked % reportingIntervalSeconds) == 0) + { + JLOG(journal_.warning) + << "Server stalled for " + << timeSpentDeadlocked << " seconds."; + } + + // If we go over 500 seconds spent deadlocked, it means that + // the deadlock resolution code has failed, which qualifies + // as undefined behavior. + // + assert (timeSpentDeadlocked < 500); + } + } + + bool change = false; + if (app_.getJobQueue ().isOverloaded ()) + { + JLOG(journal_.info) << app_.getJobQueue ().getJson (0); + change = app_.getFeeTrack ().raiseLocalFee (); + } + else + { + change = app_.getFeeTrack ().lowerLocalFee (); + } + + if (change) + { + // VFALCO TODO replace this with a Listener / observer and + // subscribe in NetworkOPs or Application. + app_.getOPs ().reportFeeChange (); + } + + t += std::chrono::seconds (1); + auto const duration = t - clock_type::now(); + + if ((duration < std::chrono::seconds (0)) || + (duration > std::chrono::seconds (1))) + { + JLOG(journal_.warning) << "time jump"; + t = clock_type::now(); + } + else + { + std::this_thread::sleep_for (duration); + } + } + + stopped (); } //------------------------------------------------------------------------------ @@ -216,7 +200,7 @@ std::unique_ptr make_LoadManager (Application& app, beast::Stoppable& parent, beast::Journal journal) { - return std::make_unique(app, parent, journal); + return std::make_unique(app, parent, journal); } } // ripple diff --git a/src/ripple/app/main/LoadManager.h b/src/ripple/app/main/LoadManager.h index 1c19e0f28a..3e083367ac 100644 --- a/src/ripple/app/main/LoadManager.h +++ b/src/ripple/app/main/LoadManager.h @@ -20,12 +20,15 @@ #ifndef RIPPLE_APP_MAIN_LOADMANAGER_H_INCLUDED #define RIPPLE_APP_MAIN_LOADMANAGER_H_INCLUDED -#include #include #include // +#include +#include namespace ripple { +class Application; + /** Manages load sources. This object creates an associated thread to maintain a clock. @@ -39,15 +42,28 @@ namespace ripple { */ class LoadManager : public beast::Stoppable { -protected: - explicit LoadManager (Stoppable& parent); +public: + // It would be better if the LoadManager constructor could be private + // with std::make_unique as a friend. But Visual Studio can't currently + // swallow the following friend declaration (Microsoft (R) C/C++ + // Optimizing Compiler Version 19.00.23026 for x64). So we make the + // constructor public. +// template +// friend std::unique_ptr std::make_unique (Args&&... args); + + // Should only be constructible by std::make_unique. + LoadManager (Application& app, Stoppable& parent, beast::Journal journal); public: + LoadManager () = delete; + LoadManager (LoadManager const&) = delete; + LoadManager& operator=(LoadManager const&) = delete; + /** Destroy the manager. The destructor returns only after the thread has stopped. */ - virtual ~LoadManager () = default; + ~LoadManager (); /** Turn on deadlock detection. @@ -57,23 +73,46 @@ public: @see resetDeadlockDetector */ - // VFALCO NOTE it seems that the deadlock detector has an "armed" state to prevent it - // from going off during program startup if there's a lengthy initialization - // operation taking place? + // VFALCO NOTE it seems that the deadlock detector has an "armed" state + // to prevent it from going off during program startup if + // there's a lengthy initialization operation taking place? // - virtual void activateDeadlockDetector () = 0; + void activateDeadlockDetector (); /** Reset the deadlock detection timer. A dedicated thread monitors the deadlock timer, and if too much time passes it will produce log warnings. */ - virtual void resetDeadlockDetector () = 0; + void resetDeadlockDetector (); + + //-------------------------------------------------------------------------- + + // Stoppable members + void onPrepare () override; + + void onStart () override; + + void onStop () override; + +private: + void run (); + +private: + Application& app_; + beast::Journal journal_; + + std::thread thread_; + std::mutex mutex_; // Guards deadLock_, armed_, and stop_. + + int deadLock_; // Detect server deadlocks. + bool armed_; + bool stop_; }; std::unique_ptr -make_LoadManager (Application& app, - beast::Stoppable& parent, beast::Journal journal); +make_LoadManager ( + Application& app, beast::Stoppable& parent, beast::Journal journal); } // ripple