From acfa83d9d000abd263d8cb5ac3355cfd8cf49ec0 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Wed, 23 Jul 2025 22:06:37 +0100 Subject: [PATCH] log: make m_limiter a shared_ptr This allows us to safely and explicitly manage the dual dependency on the limiter: one for the Logger, and one for the CScheduler. Github-Pull: #33011 Rebased-From: 3d630c2544e19480268426cda245796d4ce34ac3 --- src/init.cpp | 2 +- src/logging.cpp | 17 ++++++++++++----- src/logging.h | 11 ++++++++--- src/test/logging_tests.cpp | 8 +++++--- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 9e9cb5d732f..fa7ac6077d2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1384,7 +1384,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } }, std::chrono::minutes{5}); - LogInstance().SetRateLimiting(std::make_unique( + LogInstance().SetRateLimiting(BCLog::LogRateLimiter::Create( [&scheduler](auto func, auto window) { scheduler.scheduleEvery(std::move(func), window); }, BCLog::RATELIMIT_MAX_BYTES, BCLog::RATELIMIT_WINDOW)); diff --git a/src/logging.cpp b/src/logging.cpp index 0cad2905047..2ed6835197b 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -371,12 +371,19 @@ static size_t MemUsage(const BCLog::Logger::BufferedLog& buflog) memusage::MallocUsage(sizeof(memusage::list_node)); } -BCLog::LogRateLimiter::LogRateLimiter( - SchedulerFunction scheduler_func, - uint64_t max_bytes, - std::chrono::seconds reset_window) : m_max_bytes{max_bytes}, m_reset_window{reset_window} +BCLog::LogRateLimiter::LogRateLimiter(uint64_t max_bytes, std::chrono::seconds reset_window) + : m_max_bytes{max_bytes}, m_reset_window{reset_window} {} + +std::shared_ptr BCLog::LogRateLimiter::Create( + SchedulerFunction&& scheduler_func, uint64_t max_bytes, std::chrono::seconds reset_window) { - scheduler_func([this] { Reset(); }, reset_window); + auto limiter{std::shared_ptr(new LogRateLimiter(max_bytes, reset_window))}; + std::weak_ptr weak_limiter{limiter}; + auto reset = [weak_limiter] { + if (auto shared_limiter{weak_limiter.lock()}) shared_limiter->Reset(); + }; + scheduler_func(reset, limiter->m_reset_window); + return limiter; } BCLog::LogRateLimiter::Status BCLog::LogRateLimiter::Consume( diff --git a/src/logging.h b/src/logging.h index 04e6e0974c4..9419e245bdf 100644 --- a/src/logging.h +++ b/src/logging.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -130,6 +131,7 @@ namespace BCLog { std::unordered_map m_source_locations GUARDED_BY(m_mutex); //! Whether any log locations are suppressed. Cached view on m_source_locations for performance reasons. std::atomic m_suppression_active{false}; + LogRateLimiter(uint64_t max_bytes, std::chrono::seconds reset_window); public: using SchedulerFunction = std::function, std::chrono::milliseconds)>; @@ -141,7 +143,10 @@ namespace BCLog { * location. * @param reset_window Time window after which the stats are reset. */ - LogRateLimiter(SchedulerFunction scheduler_func, uint64_t max_bytes, std::chrono::seconds reset_window); + static std::shared_ptr Create( + SchedulerFunction&& scheduler_func, + uint64_t max_bytes, + std::chrono::seconds reset_window); //! Maximum number of bytes logged per location per window. const uint64_t m_max_bytes; //! Interval after which the window is reset. @@ -186,7 +191,7 @@ namespace BCLog { size_t m_buffer_lines_discarded GUARDED_BY(m_cs){0}; //! Manages the rate limiting of each log location. - std::unique_ptr m_limiter GUARDED_BY(m_cs); + std::shared_ptr m_limiter GUARDED_BY(m_cs); //! Category-specific log level. Overrides `m_log_level`. std::unordered_map m_category_log_levels GUARDED_BY(m_cs); @@ -255,7 +260,7 @@ namespace BCLog { /** Only for testing */ void DisconnectTestLogger() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); - void SetRateLimiting(std::unique_ptr&& limiter) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) + void SetRateLimiting(std::shared_ptr limiter) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); m_limiter = std::move(limiter); diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 3fd66470246..41c0b1dd32f 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -69,6 +69,7 @@ struct LogSetup : public BasicTestingSetup { LogInstance().SetLogLevel(BCLog::Level::Debug); LogInstance().SetCategoryLogLevel({}); + LogInstance().SetRateLimiting(nullptr); } ~LogSetup() @@ -82,6 +83,7 @@ struct LogSetup : public BasicTestingSetup { LogInstance().m_log_sourcelocations = prev_log_sourcelocations; LogInstance().SetLogLevel(prev_log_level); LogInstance().SetCategoryLogLevel(prev_category_levels); + LogInstance().SetRateLimiting(nullptr); } }; @@ -309,7 +311,8 @@ BOOST_AUTO_TEST_CASE(logging_log_rate_limiter) uint64_t max_bytes{1024}; auto reset_window{1min}; auto sched_func = [&scheduler](auto func, auto window) { scheduler.scheduleEvery(std::move(func), window); }; - BCLog::LogRateLimiter limiter{sched_func, max_bytes, reset_window}; + auto limiter_{BCLog::LogRateLimiter::Create(sched_func, max_bytes, reset_window)}; + auto& limiter{*limiter_}; using Status = BCLog::LogRateLimiter::Status; auto source_loc_1{std::source_location::current()}; @@ -405,8 +408,7 @@ BOOST_FIXTURE_TEST_CASE(logging_filesize_rate_limit, LogSetup) CScheduler scheduler{}; scheduler.m_service_thread = std::thread([&] { scheduler.serviceQueue(); }); auto sched_func = [&scheduler](auto func, auto window) { scheduler.scheduleEvery(std::move(func), window); }; - auto limiter = std::make_unique(sched_func, 1024 * 1024, 20s); - LogInstance().SetRateLimiting(std::move(limiter)); + LogInstance().SetRateLimiting(BCLog::LogRateLimiter::Create(sched_func, 1024 * 1024, 20s)); // Log 1024-character lines (1023 plus newline) to make the math simple. std::string log_message(1023, 'a');