diff options
author | MacroFake <falke.marco@gmail.com> | 2022-05-04 10:54:19 +0200 |
---|---|---|
committer | MacroFake <falke.marco@gmail.com> | 2022-05-04 10:54:26 +0200 |
commit | 14cb53dfe95ecf777afa85430cede609fb5379b4 (patch) | |
tree | 237048aa63c7c7da5d1d0707d59f209097d1bdbd /src | |
parent | 9b42d62f426fd22602e4e9a589ab000e0f29a65b (diff) | |
parent | fa4652ce5995ace831b6a4d3125bfcac9563ff6f (diff) | |
download | bitcoin-14cb53dfe95ecf777afa85430cede609fb5379b4.tar.xz |
Merge bitcoin/bitcoin#25040: refactor: Pass lifetimebound reference to SingleThreadedSchedulerClient
fa4652ce5995ace831b6a4d3125bfcac9563ff6f Pass lifetimebound reference to SingleThreadedSchedulerClient (MacroFake)
Pull request description:
Currently a pointer is passed, which is confusing and requires run-time asserts to avoid nullptr dereference.
All call sites can pass a reference, so do that. Also mark it LIFETIMEBOUND to avoid call sites passing a temporary. Also, unrelated cleanup in touched lines.
ACKs for top commit:
pk-b2:
ACK https://github.com/bitcoin/bitcoin/pull/25040/commits/fa4652ce5995ace831b6a4d3125bfcac9563ff6f
jonatack:
Code review ACK fa4652ce5995ace831b6a4d3125bfcac9563ff6f rebased to master, debug build, unit tests
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/25040/commits/fa4652ce5995ace831b6a4d3125bfcac9563ff6f
Tree-SHA512: cd7ec77347e195d659b8892d34c1e9644d4f88552a4d5fa310dc1756eb27050a99d3098b0b0d27f8474230f82c178fd9e22e7018d8248d5e47a7f4caad395e25
Diffstat (limited to 'src')
-rw-r--r-- | src/scheduler.cpp | 6 | ||||
-rw-r--r-- | src/scheduler.h | 13 | ||||
-rw-r--r-- | src/test/scheduler_tests.cpp | 6 | ||||
-rw-r--r-- | src/validationinterface.cpp | 4 |
4 files changed, 16 insertions, 13 deletions
diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 197d009f7c..3e7ee7d370 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -143,7 +143,7 @@ void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue() if (m_are_callbacks_running) return; if (m_callbacks_pending.empty()) return; } - m_pscheduler->schedule(std::bind(&SingleThreadedSchedulerClient::ProcessQueue, this), std::chrono::system_clock::now()); + m_scheduler.schedule([this] { this->ProcessQueue(); }, std::chrono::system_clock::now()); } void SingleThreadedSchedulerClient::ProcessQueue() @@ -179,8 +179,6 @@ void SingleThreadedSchedulerClient::ProcessQueue() void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void()> func) { - assert(m_pscheduler); - { LOCK(m_callbacks_mutex); m_callbacks_pending.emplace_back(std::move(func)); @@ -190,7 +188,7 @@ void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void()> func void SingleThreadedSchedulerClient::EmptyQueue() { - assert(!m_pscheduler->AreThreadsServicingQueue()); + assert(!m_scheduler.AreThreadsServicingQueue()); bool should_continue = true; while (should_continue) { ProcessQueue(); diff --git a/src/scheduler.h b/src/scheduler.h index bb0abfbf7a..eb350e4bc3 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -5,13 +5,18 @@ #ifndef BITCOIN_SCHEDULER_H #define BITCOIN_SCHEDULER_H +#include <attributes.h> +#include <sync.h> +#include <threadsafety.h> + +#include <chrono> #include <condition_variable> +#include <cstddef> #include <functional> #include <list> #include <map> #include <thread> - -#include <sync.h> +#include <utility> /** * Simple class for background tasks that should be run @@ -117,7 +122,7 @@ private: class SingleThreadedSchedulerClient { private: - CScheduler* m_pscheduler; + CScheduler& m_scheduler; Mutex m_callbacks_mutex; std::list<std::function<void()>> m_callbacks_pending GUARDED_BY(m_callbacks_mutex); @@ -127,7 +132,7 @@ private: void ProcessQueue(); public: - explicit SingleThreadedSchedulerClient(CScheduler* pschedulerIn) : m_pscheduler(pschedulerIn) {} + explicit SingleThreadedSchedulerClient(CScheduler& scheduler LIFETIMEBOUND) : m_scheduler{scheduler} {} /** * Add a callback to be executed. Callbacks are executed serially diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index 4195d413fc..6e089de0c1 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -128,8 +128,8 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) CScheduler scheduler; // each queue should be well ordered with respect to itself but not other queues - SingleThreadedSchedulerClient queue1(&scheduler); - SingleThreadedSchedulerClient queue2(&scheduler); + SingleThreadedSchedulerClient queue1(scheduler); + SingleThreadedSchedulerClient queue2(scheduler); // create more threads than queues // if the queues only permit execution of one task at once then @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) // if they don't we'll get out of order behaviour std::vector<std::thread> threads; for (int i = 0; i < 5; ++i) { - threads.emplace_back(std::bind(&CScheduler::serviceQueue, &scheduler)); + threads.emplace_back([&] { scheduler.serviceQueue(); }); } // these are not atomic, if SinglethreadedSchedulerClient prevents diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 1e07ff23ae..edc4633c01 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -40,7 +40,7 @@ public: // our own queue here :( SingleThreadedSchedulerClient m_schedulerClient; - explicit MainSignalsInstance(CScheduler *pscheduler) : m_schedulerClient(pscheduler) {} + explicit MainSignalsInstance(CScheduler& scheduler LIFETIMEBOUND) : m_schedulerClient(scheduler) {} void Register(std::shared_ptr<CValidationInterface> callbacks) { @@ -92,7 +92,7 @@ static CMainSignals g_signals; void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler& scheduler) { assert(!m_internals); - m_internals.reset(new MainSignalsInstance(&scheduler)); + m_internals = std::make_unique<MainSignalsInstance>(scheduler); } void CMainSignals::UnregisterBackgroundSignalScheduler() |