Fix race condition for SetBannedSetDirty() calls

hebasto · hebasto · commit 99a6b699cd65 · 2022-01-28T19:27:25.000Z
Another thread can `SetBannedSetDirty(true)` while `CBanDB::Write()`
call being executed. The following `SetBannedSetDirty(false)`
effectively makes `m_is_dirty` flag value inconsistent with the actual
`m_banned` state. Such behavior can result in data loss, e.g., during
shutdown.
diff --git a/src/banman.cpp b/src/banman.cpp
@@ -49,11 +49,12 @@ void BanMan::DumpBanlist()
         SweepBanned();
         if (!BannedSetIsDirty()) return;
         banmap = m_banned;
+        SetBannedSetDirty(false);
     }
 
     int64_t n_start = GetTimeMillis();
-    if (m_ban_db.Write(banmap)) {
-        SetBannedSetDirty(false);
+    if (!m_ban_db.Write(banmap)) {
+        SetBannedSetDirty(true);
     }
 
     LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk  %dms\n", banmap.size(),

Original file line number	Diff line number	Diff line change
`@@ -49,11 +49,12 @@ void BanMan::DumpBanlist()`
`49`	`49`	`SweepBanned();`
`50`	`50`	`if (!BannedSetIsDirty()) return;`
`51`	`51`	`banmap = m_banned;`
	`52`	`+ SetBannedSetDirty(false);`
`52`	`53`	`}`
`53`	`54`
`54`	`55`	`int64_t n_start = GetTimeMillis();`
`55`		`- if (m_ban_db.Write(banmap)) {`
`56`		`- SetBannedSetDirty(false);`
	`56`	`+ if (!m_ban_db.Write(banmap)) {`
	`57`	`+ SetBannedSetDirty(true);`
`57`	`58`	`}`
`58`	`59`
`59`	`60`	`LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk %dms\n", banmap.size(),`