Skip to content

Commit c231d4d

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 accumulating redundant scheduled tasks: Ban() only reschedules when the new ban expires sooner than what is already scheduled, and ScheduleNextSweep() skips if a sweep is already pending for an earlier or equal time. Fix an off-by-one in SweepBanned(): use >= instead of > so that entries are swept at the exact expiry second, consistent with IsBanned() which uses < for the active check. This prevents a tight reschedule loop when the scheduler fires at the boundary. Fixes bitcoinknots#273
1 parent a9aee73 commit c231d4d

3 files changed

Lines changed: 57 additions & 1 deletion

File tree

src/banman.cpp

Lines changed: 48 additions & 1 deletion
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()
41+
{
42+
LOCK(m_banned_mutex);
43+
SweepBanned();
44+
m_next_sweep_time = std::numeric_limits<int64_t>::max();
45+
ScheduleNextSweep();
46+
}
47+
48+
void BanMan::ScheduleNextSweep()
49+
{
50+
AssertLockHeld(m_banned_mutex);
51+
52+
if (!m_scheduler) return;
53+
if (m_banned.empty()) return;
54+
55+
int64_t earliest = std::numeric_limits<int64_t>::max();
56+
for (const auto& [subnet, entry] : m_banned) {
57+
if (entry.nBanUntil < earliest) {
58+
earliest = entry.nBanUntil;
59+
}
60+
}
61+
62+
if (earliest >= m_next_sweep_time) return;
63+
64+
m_next_sweep_time = earliest;
65+
int64_t now = GetTime();
66+
auto delay = std::chrono::seconds(std::max<int64_t>(0, earliest - now));
67+
m_scheduler->scheduleFromNow([this] { SweepBannedAndSchedule(); }, delay);
68+
}
69+
2970
void BanMan::LoadBanlist()
3071
{
3172
LOCK(m_banned_mutex);
@@ -144,6 +185,12 @@ 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+
int64_t now = GetTime();
191+
auto delay = std::chrono::seconds(std::max<int64_t>(0, m_next_sweep_time - now));
192+
m_scheduler->scheduleFromNow([this] { SweepBannedAndSchedule(); }, delay);
193+
}
147194
} else
148195
return;
149196
}
@@ -189,7 +236,7 @@ void BanMan::SweepBanned()
189236
while (it != m_banned.end()) {
190237
CSubNet sub_net = (*it).first;
191238
CBanEntry ban_entry = (*it).second;
192-
if (!sub_net.IsValid() || now > ban_entry.nBanUntil) {
239+
if (!sub_net.IsValid() || now >= ban_entry.nBanUntil) {
193240
m_banned.erase(it++);
194241
m_is_dirty = true;
195242
notify_ui = true;

src/banman.h

Lines changed: 7 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,12 @@ 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() 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()};
8895
banmap_t m_banned GUARDED_BY(m_banned_mutex);
8996
bool m_is_dirty GUARDED_BY(m_banned_mutex){false};
9097
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)