Skip to content

Commit 87325b4

Browse files
committed
banman: schedule sweep at ban expiry instead of polling
Give BanMan access to CScheduler so it can fire SweepBanned() at the exact moment the next ban expires. This triggers BannedListChanged to refresh the GUI without polling timers. Track m_next_sweep_time to avoid scheduling when a sweep is already pending for an earlier time. Ban() compares the new ban's expiry directly against the tracked time (O(1), no loop). Use a generation counter (m_sweep_seq) so that when a new ban moves the scheduled time forward, all previously-queued callbacks are invalidated. Stale callbacks check their captured sequence number against the current one and bail immediately without locking or sweeping. Fixes bitcoinknots#273
1 parent 13398b9 commit 87325b4

3 files changed

Lines changed: 58 additions & 0 deletions

File tree

src/banman.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@
99
#include <logging.h>
1010
#include <netaddress.h>
1111
#include <node/interface_ui.h>
12+
#include <scheduler.h>
1213
#include <sync.h>
1314
#include <util/time.h>
1415
#include <util/translation.h>
1516

17+
#include <algorithm>
18+
#include <limits>
19+
1620

1721
BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time)
1822
: m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time)
@@ -26,6 +30,43 @@ BanMan::~BanMan()
2630
DumpBanlist();
2731
}
2832

33+
void BanMan::StartScheduledTasks(CScheduler& scheduler)
34+
{
35+
LOCK(m_banned_mutex);
36+
m_scheduler = &scheduler;
37+
ScheduleNextSweep();
38+
}
39+
40+
void BanMan::SweepBannedAndSchedule(uint64_t expected_seq)
41+
{
42+
LOCK(m_banned_mutex);
43+
if (expected_seq != m_sweep_seq) return;
44+
SweepBanned();
45+
m_next_sweep_time = std::numeric_limits<int64_t>::max();
46+
ScheduleNextSweep();
47+
}
48+
49+
void BanMan::ScheduleNextSweep()
50+
{
51+
AssertLockHeld(m_banned_mutex);
52+
53+
if (!m_scheduler) return;
54+
if (m_banned.empty()) return;
55+
56+
int64_t earliest = std::numeric_limits<int64_t>::max();
57+
for (const auto& [subnet, entry] : m_banned) {
58+
if (entry.nBanUntil < earliest) {
59+
earliest = entry.nBanUntil;
60+
}
61+
}
62+
63+
m_next_sweep_time = earliest;
64+
uint64_t seq = ++m_sweep_seq;
65+
int64_t now = GetTime();
66+
auto delay = std::chrono::seconds(std::max<int64_t>(0, earliest - now));
67+
m_scheduler->scheduleFromNow([this, seq] { SweepBannedAndSchedule(seq); }, delay);
68+
}
69+
2970
void BanMan::LoadBanlist()
3071
{
3172
LOCK(m_banned_mutex);
@@ -144,6 +185,13 @@ void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_uni
144185
if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) {
145186
m_banned[sub_net] = ban_entry;
146187
m_is_dirty = true;
188+
if (m_scheduler && ban_entry.nBanUntil < m_next_sweep_time) {
189+
m_next_sweep_time = ban_entry.nBanUntil;
190+
uint64_t seq = ++m_sweep_seq;
191+
int64_t now = GetTime();
192+
auto delay = std::chrono::seconds(std::max<int64_t>(0, m_next_sweep_time - now));
193+
m_scheduler->scheduleFromNow([this, seq] { SweepBannedAndSchedule(seq); }, delay);
194+
}
147195
} else
148196
return;
149197
}

src/banman.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include <chrono>
1515
#include <cstdint>
16+
#include <limits>
1617
#include <memory>
1718

1819
// NOTE: When adjusting this, update rpcnet:setban's help ("24h")
@@ -23,6 +24,7 @@ static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15};
2324

2425
class CClientUIInterface;
2526
class CNetAddr;
27+
class CScheduler;
2628
class CSubNet;
2729

2830
// Banman manages two related but distinct concepts:
@@ -62,6 +64,7 @@ class BanMan
6264
BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
6365
void Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
6466
void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
67+
void StartScheduledTasks(CScheduler& scheduler) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
6568
void Discourage(const CNetAddr& net_addr) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
6669
void ClearBanned() EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
6770

@@ -83,8 +86,13 @@ class BanMan
8386
void LoadBanlist() EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
8487
//!clean unused entries (if bantime has expired)
8588
void SweepBanned() EXCLUSIVE_LOCKS_REQUIRED(m_banned_mutex);
89+
void SweepBannedAndSchedule(uint64_t expected_seq) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
90+
void ScheduleNextSweep() EXCLUSIVE_LOCKS_REQUIRED(m_banned_mutex);
8691

8792
Mutex m_banned_mutex;
93+
CScheduler* m_scheduler GUARDED_BY(m_banned_mutex){nullptr};
94+
int64_t m_next_sweep_time GUARDED_BY(m_banned_mutex){std::numeric_limits<int64_t>::max()};
95+
uint64_t m_sweep_seq GUARDED_BY(m_banned_mutex){0};
8896
banmap_t m_banned GUARDED_BY(m_banned_mutex);
8997
bool m_is_dirty GUARDED_BY(m_banned_mutex){false};
9098
CClientUIInterface* m_client_interface = nullptr;

src/init.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2437,6 +2437,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
24372437
banman->DumpBanlist();
24382438
}, DUMP_BANS_INTERVAL);
24392439

2440+
banman->StartScheduledTasks(scheduler);
2441+
24402442
if (node.peerman) node.peerman->StartScheduledTasks(scheduler);
24412443

24422444
#if HAVE_SYSTEM

0 commit comments

Comments
 (0)