What the developer wrote
Authored by stickies-v
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.
What changed, and why it matters
This commit changes how Bitcoin Core's log rate-limiter is managed in memory. Previously, the scheduler held a raw pointer to the limiter, which could become invalid if the limiter was destroyed while a scheduled reset task was still pending. The fix makes the limiter a shared pointer and gives the scheduler only a weak reference, so the scheduled task safely does nothing if the limiter is gone. This is a defensive hardening change, not an active exploit fix.
Treat as a low-risk hardening improvement. Review related shutdown ordering and scheduler callback lifetime patterns elsewhere in the codebase. No urgent deployment action is indicated by this commit alone.
Security signals we found
Change in object lifetime management (unique_ptr -> shared_ptr + weak_ptr)
Scheduled callback now checks object validity before use
Defensive fix for potential dangling reference between CScheduler and Logger
No explicit security claim or CVE in commit message
Evidence from the diff
The patch refactors BCLog::LogRateLimiter ownership from std::unique_ptr to std::shared_ptr. The constructor no longer registers a scheduler callback directly; instead a static Create() factory builds a shared_ptr, captures a std::weak_ptr in the scheduled reset lambda, and only calls Reset() if the object still exists. Logger::m_limiter and SetRateLimiting() are updated accordingly. Tests are adjusted to use the factory and to reset the limiter in LogSetup. The change eliminates a potential use-after-free or dangling-pointer scenario where CScheduler could invoke a callback on a destroyed LogRateLimiter.
Changed components
src/logging.cppsrc/logging.hsrc/init.cppsrc/test/logging_tests.cppBCLog::LogRateLimiterBCLog::Logger::SetRateLimitingCScheduler integration in initInspect captured patch +27 / −13
diff --git a/src/init.cpp b/src/init.cpp
index b48d3cc0..297910e2 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1381,7 +1381,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
}
}, std::chrono::minutes{5});
- LogInstance().SetRateLimiting(std::make_unique<BCLog::LogRateLimiter>(
+ 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 0cad2905..2ed68351 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::Logger::BufferedLog>));
}
-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}
-{
- scheduler_func([this] { Reset(); }, 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> BCLog::LogRateLimiter::Create(
+ SchedulerFunction&& scheduler_func, uint64_t max_bytes, std::chrono::seconds reset_window)
+{
+ auto limiter{std::shared_ptr<LogRateLimiter>(new LogRateLimiter(max_bytes, reset_window))};
+ std::weak_ptr<LogRateLimiter> 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 04e6e097..9419e245 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -18,6 +18,7 @@
#include <cstring>
#include <functional>
#include <list>
+#include <memory>
#include <mutex>
#include <source_location>
#include <string>
@@ -130,6 +131,7 @@ namespace BCLog {
std::unordered_map<std::source_location, Stats, SourceLocationHasher, SourceLocationEqual> m_source_locations GUARDED_BY(m_mutex);
//! Whether any log locations are suppressed. Cached view on m_source_locations for performance reasons.
std::atomic<bool> m_suppression_active{false};
+ LogRateLimiter(uint64_t max_bytes, std::chrono::seconds reset_window);
public:
using SchedulerFunction = std::function<void(std::function<void()>, 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<LogRateLimiter> 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<LogRateLimiter> m_limiter GUARDED_BY(m_cs);
+ std::shared_ptr<LogRateLimiter> m_limiter GUARDED_BY(m_cs);
//! Category-specific log level. Overrides `m_log_level`.
std::unordered_map<LogFlags, Level> 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<LogRateLimiter>&& limiter) EXCLUSIVE_LOCKS_REQUIRED(!m_cs)
+ void SetRateLimiting(std::shared_ptr<LogRateLimiter> 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 dbe13886..4d4141c1 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<BCLog::LogRateLimiter>(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');
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.