From fa70ccc6c4e304646b4610228f3975b3a9762643 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 6 Mar 2020 17:35:09 -0500 Subject: scheduler: Use C++11 member initialization, add shutdown assert "Initializing the members in the declaration makes it easy to spot uninitialized ones". https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures --- src/scheduler.cpp | 3 ++- src/scheduler.h | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 7cb7754fde..67e17d074b 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -9,13 +9,14 @@ #include #include -CScheduler::CScheduler() : nThreadsServicingQueue(0), stopRequested(false), stopWhenEmpty(false) +CScheduler::CScheduler() { } CScheduler::~CScheduler() { assert(nThreadsServicingQueue == 0); + if (stopWhenEmpty) assert(taskQueue.empty()); } diff --git a/src/scheduler.h b/src/scheduler.h index 4d5aa3068e..71d1726c4f 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -86,9 +86,9 @@ private: mutable Mutex newTaskMutex; std::condition_variable newTaskScheduled; std::multimap taskQueue GUARDED_BY(newTaskMutex); - int nThreadsServicingQueue GUARDED_BY(newTaskMutex); - bool stopRequested GUARDED_BY(newTaskMutex); - bool stopWhenEmpty GUARDED_BY(newTaskMutex); + int nThreadsServicingQueue GUARDED_BY(newTaskMutex){0}; + bool stopRequested GUARDED_BY(newTaskMutex){false}; + bool stopWhenEmpty GUARDED_BY(newTaskMutex){false}; bool shouldStop() const EXCLUSIVE_LOCKS_REQUIRED(newTaskMutex) { return stopRequested || (stopWhenEmpty && taskQueue.empty()); } }; -- cgit v1.2.3 From fadafb83cff9a9a340eac1b5a853e2467d5e0ef7 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 6 Mar 2020 18:06:50 -0500 Subject: scheduler: Make schedule* methods type safe --- src/init.cpp | 8 ++++---- src/net.cpp | 6 +++--- src/net_processing.cpp | 2 +- src/scheduler.cpp | 13 ++++--------- src/scheduler.h | 24 ++++++++++++++---------- src/test/scheduler_tests.cpp | 12 ++++++------ src/wallet/load.cpp | 4 ++-- 7 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a637aac4d2..eaabcf7382 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -86,8 +86,8 @@ static const bool DEFAULT_PROXYRANDOMIZE = true; static const bool DEFAULT_REST_ENABLE = false; static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false; -// Dump addresses to banlist.dat every 15 minutes (900s) -static constexpr int DUMP_BANS_INTERVAL = 60 * 15; +// How often to dump addresses to banlist.dat +static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15}; #ifdef WIN32 @@ -1278,7 +1278,7 @@ bool AppInitMain(NodeContext& node) // Gather some entropy once per minute. node.scheduler->scheduleEvery([]{ RandAddPeriodic(); - }, 60000); + }, std::chrono::minutes{1}); GetMainSignals().RegisterBackgroundSignalScheduler(*node.scheduler); @@ -1863,7 +1863,7 @@ bool AppInitMain(NodeContext& node) BanMan* banman = node.banman.get(); node.scheduler->scheduleEvery([banman]{ banman->DumpBanlist(); - }, DUMP_BANS_INTERVAL * 1000); + }, DUMP_BANS_INTERVAL); return true; } diff --git a/src/net.cpp b/src/net.cpp index d156450394..8352c40b98 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -45,8 +45,8 @@ static_assert(MINIUPNPC_API_VERSION >= 10, "miniUPnPc API version >= 10 assumed" #include -// Dump addresses to peers.dat every 15 minutes (900s) -static constexpr int DUMP_PEERS_INTERVAL = 15 * 60; +// How often to dump addresses to peers.dat +static constexpr std::chrono::minutes DUMP_PEERS_INTERVAL{15}; /** Number of DNS seeds to query when the number of connections is low. */ static constexpr int DNSSEEDS_TO_QUERY_AT_ONCE = 3; @@ -2343,7 +2343,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) threadMessageHandler = std::thread(&TraceThread >, "msghand", std::function(std::bind(&CConnman::ThreadMessageHandler, this))); // Dump network addresses - scheduler.scheduleEvery(std::bind(&CConnman::DumpAddresses, this), DUMP_PEERS_INTERVAL * 1000); + scheduler.scheduleEvery([this] { DumpAddresses(); }, DUMP_PEERS_INTERVAL); return true; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fa09b60630..b9f77685ab 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1124,7 +1124,7 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CS // combine them in one function and schedule at the quicker (peer-eviction) // timer. static_assert(EXTRA_PEER_CHECK_INTERVAL < STALE_CHECK_INTERVAL, "peer eviction timer should be less than stale tip check timer"); - scheduler.scheduleEvery(std::bind(&PeerLogicValidation::CheckForStaleTipAndEvictPeers, this, consensusParams), EXTRA_PEER_CHECK_INTERVAL * 1000); + scheduler.scheduleEvery([&] { this->CheckForStaleTipAndEvictPeers(consensusParams); }, std::chrono::seconds{EXTRA_PEER_CHECK_INTERVAL}); } /** diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 67e17d074b..4cac5a54e0 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -92,11 +92,6 @@ void CScheduler::schedule(CScheduler::Function f, std::chrono::system_clock::tim newTaskScheduled.notify_one(); } -void CScheduler::scheduleFromNow(CScheduler::Function f, int64_t deltaMilliSeconds) -{ - schedule(f, std::chrono::system_clock::now() + std::chrono::milliseconds(deltaMilliSeconds)); -} - void CScheduler::MockForward(std::chrono::seconds delta_seconds) { assert(delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}); @@ -119,15 +114,15 @@ void CScheduler::MockForward(std::chrono::seconds delta_seconds) newTaskScheduled.notify_one(); } -static void Repeat(CScheduler* s, CScheduler::Function f, int64_t deltaMilliSeconds) +static void Repeat(CScheduler& s, CScheduler::Function f, std::chrono::milliseconds delta) { f(); - s->scheduleFromNow(std::bind(&Repeat, s, f, deltaMilliSeconds), deltaMilliSeconds); + s.scheduleFromNow([=, &s] { Repeat(s, f, delta); }, delta); } -void CScheduler::scheduleEvery(CScheduler::Function f, int64_t deltaMilliSeconds) +void CScheduler::scheduleEvery(CScheduler::Function f, std::chrono::milliseconds delta) { - scheduleFromNow(std::bind(&Repeat, this, f, deltaMilliSeconds), deltaMilliSeconds); + scheduleFromNow([=] { Repeat(*this, f, delta); }, delta); } size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point &first, diff --git a/src/scheduler.h b/src/scheduler.h index 71d1726c4f..1e64195484 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -24,8 +24,8 @@ // Usage: // // CScheduler* s = new CScheduler(); -// s->scheduleFromNow(doSomething, 11); // Assuming a: void doSomething() { } -// s->scheduleFromNow(std::bind(Class::func, this, argument), 3); +// s->scheduleFromNow(doSomething, std::chrono::milliseconds{11}); // Assuming a: void doSomething() { } +// s->scheduleFromNow([=] { this->func(argument); }, std::chrono::milliseconds{3}); // boost::thread* t = new boost::thread(std::bind(CScheduler::serviceQueue, s)); // // ... then at program shutdown, make sure to call stop() to clean up the thread(s) running serviceQueue: @@ -46,15 +46,19 @@ public: // Call func at/after time t void schedule(Function f, std::chrono::system_clock::time_point t); - // Convenience method: call f once deltaMilliSeconds from now - void scheduleFromNow(Function f, int64_t deltaMilliSeconds); + /** Call f once after the delta has passed */ + void scheduleFromNow(Function f, std::chrono::milliseconds delta) + { + schedule(std::move(f), std::chrono::system_clock::now() + delta); + } - // Another convenience method: call f approximately - // every deltaMilliSeconds forever, starting deltaMilliSeconds from now. - // To be more precise: every time f is finished, it - // is rescheduled to run deltaMilliSeconds later. If you - // need more accurate scheduling, don't use this method. - void scheduleEvery(Function f, int64_t deltaMilliSeconds); + /** + * Repeat f until the scheduler is stopped. First run is after delta has passed once. + * + * The timing is not exact: Every time f is finished, it is rescheduled to run again after delta. If you need more + * accurate scheduling, don't use this method. + */ + void scheduleEvery(Function f, std::chrono::milliseconds delta); /** * Mock the scheduler to fast forward in time. diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index 7d26840b73..fb95802b67 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -150,10 +150,10 @@ BOOST_AUTO_TEST_CASE(mockforward) CScheduler::Function dummy = [&counter]{counter++;}; // schedule jobs for 2, 5 & 8 minutes into the future - int64_t min_in_milli = 60*1000; - scheduler.scheduleFromNow(dummy, 2*min_in_milli); - scheduler.scheduleFromNow(dummy, 5*min_in_milli); - scheduler.scheduleFromNow(dummy, 8*min_in_milli); + + scheduler.scheduleFromNow(dummy, std::chrono::minutes{2}); + scheduler.scheduleFromNow(dummy, std::chrono::minutes{5}); + scheduler.scheduleFromNow(dummy, std::chrono::minutes{8}); // check taskQueue std::chrono::system_clock::time_point first, last; @@ -163,10 +163,10 @@ BOOST_AUTO_TEST_CASE(mockforward) std::thread scheduler_thread([&]() { scheduler.serviceQueue(); }); // bump the scheduler forward 5 minutes - scheduler.MockForward(std::chrono::seconds(5*60)); + scheduler.MockForward(std::chrono::minutes{5}); // ensure scheduler has chance to process all tasks queued for before 1 ms from now. - scheduler.scheduleFromNow([&scheduler]{ scheduler.stop(false); }, 1); + scheduler.scheduleFromNow([&scheduler] { scheduler.stop(false); }, std::chrono::milliseconds{1}); scheduler_thread.join(); // check that the queue only has one job remaining diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 071befaebf..3e92c07d64 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -88,8 +88,8 @@ void StartWallets(CScheduler& scheduler) } // Schedule periodic wallet flushes and tx rebroadcasts - scheduler.scheduleEvery(MaybeCompactWalletDB, 500); - scheduler.scheduleEvery(MaybeResendWalletTxs, 1000); + scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + scheduler.scheduleEvery(MaybeResendWalletTxs, std::chrono::milliseconds{1000}); } void FlushWallets() -- cgit v1.2.3 From fa36f3a29538012a6eb5c3402b3b3c18fd32b230 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 10 Mar 2020 09:58:49 -0400 Subject: refactor: move DUMP_BANS_INTERVAL to banman.h --- src/banman.h | 9 ++++++--- src/init.cpp | 4 ---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/banman.h b/src/banman.h index 8984874914..6bea2e75e9 100644 --- a/src/banman.h +++ b/src/banman.h @@ -5,16 +5,19 @@ #ifndef BITCOIN_BANMAN_H #define BITCOIN_BANMAN_H -#include -#include - #include #include #include // For banmap_t #include +#include +#include +#include + // NOTE: When adjusting this, update rpcnet:setban's help ("24h") static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban +// How often to dump addresses to banlist.dat +static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15}; class CClientUIInterface; class CNetAddr; diff --git a/src/init.cpp b/src/init.cpp index eaabcf7382..22d5b8660a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -86,10 +86,6 @@ static const bool DEFAULT_PROXYRANDOMIZE = true; static const bool DEFAULT_REST_ENABLE = false; static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false; -// How often to dump addresses to banlist.dat -static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15}; - - #ifdef WIN32 // Win32 LevelDB doesn't use filedescriptors, and the ones used for // accessing block files don't count towards the fd_set size limit -- cgit v1.2.3