Merge bitcoin/bitcoin#24097: Replace RecursiveMutex m_cs_banned with Mutex, and rename it

37d150d8c5ffcb2bddcd99951a739e97571194c7 refactor: Add more negative `!m_banned_mutex` thread safety annotations (Hennadii Stepanov)
0fb29087080a4e60d7c709ff5edf14e830ef3a69 refactor: replace RecursiveMutex m_banned_mutex with Mutex (w0xlt)
784c316f9cb664c9577cbfed1873bae573efd1b4 scripted-diff: rename m_cs_banned -> m_banned_mutex (w0xlt)
46709c5f27bf6cbc8eba1298b04bd079da2cdded refactor: Get rid of `BanMan::SetBannedSetDirty()` (Hennadii Stepanov)
d88c0d8440cf640ef4f2c7a40b8b8b31bfd38f23 refactor: Get rid of `BanMan::BannedSetIsDirty()` (Hennadii Stepanov)

Pull request description:

  This PR is an alternative to bitcoin/bitcoin#24092. Last two commit have been cherry-picked from the latter.

ACKs for top commit:
  maflcko:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7 🎾
  achow101:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
  theStack:
    Code-review ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
  vasild:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7

Tree-SHA512: 5e9d40101a09af6e0645a6ede67432ea68631a1b960f9e6af0ad07415ca7718a30fcc1aad5182d1d5265dc54c26aba2008fc9973840255c09adbab8fedf10075

1 files changed, 15 insertions, 26 deletions

diff --git a/src/banman.cpp b/src/banman.cpp
index a96b7e3c53..9f668d76a3 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -28,7 +28,7 @@ BanMan::~BanMan()
 
 void BanMan::LoadBanlist()
 {
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
 
     if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated);
 
@@ -52,16 +52,17 @@ void BanMan::DumpBanlist()
 
     banmap_t banmap;
     {
-        LOCK(m_cs_banned);
+        LOCK(m_banned_mutex);
         SweepBanned();
-        if (!BannedSetIsDirty()) return;
+        if (!m_is_dirty) return;
         banmap = m_banned;
-        SetBannedSetDirty(false);
+        m_is_dirty = false;
     }
 
     const auto start{SteadyClock::now()};
     if (!m_ban_db.Write(banmap)) {
-        SetBannedSetDirty(true);
+        LOCK(m_banned_mutex);
+        m_is_dirty = true;
     }
 
     LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk  %dms\n", banmap.size(),
@@ -71,7 +72,7 @@ void BanMan::DumpBanlist()
 void BanMan::ClearBanned()
 {
     {
-        LOCK(m_cs_banned);
+        LOCK(m_banned_mutex);
         m_banned.clear();
         m_is_dirty = true;
     }
@@ -81,14 +82,14 @@ void BanMan::ClearBanned()
 
 bool BanMan::IsDiscouraged(const CNetAddr& net_addr)
 {
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
     return m_discouraged.contains(net_addr.GetAddrBytes());
 }
 
 bool BanMan::IsBanned(const CNetAddr& net_addr)
 {
     auto current_time = GetTime();
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
     for (const auto& it : m_banned) {
         CSubNet sub_net = it.first;
         CBanEntry ban_entry = it.second;
@@ -103,7 +104,7 @@ bool BanMan::IsBanned(const CNetAddr& net_addr)
 bool BanMan::IsBanned(const CSubNet& sub_net)
 {
     auto current_time = GetTime();
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
     banmap_t::iterator i = m_banned.find(sub_net);
     if (i != m_banned.end()) {
         CBanEntry ban_entry = (*i).second;
@@ -122,7 +123,7 @@ void BanMan::Ban(const CNetAddr& net_addr, int64_t ban_time_offset, bool since_u
 
 void BanMan::Discourage(const CNetAddr& net_addr)
 {
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
     m_discouraged.insert(net_addr.GetAddrBytes());
 }
 
@@ -139,7 +140,7 @@ void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_uni
     ban_entry.nBanUntil = (normalized_since_unix_epoch ? 0 : GetTime()) + normalized_ban_time_offset;
 
     {
-        LOCK(m_cs_banned);
+        LOCK(m_banned_mutex);
         if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) {
             m_banned[sub_net] = ban_entry;
             m_is_dirty = true;
@@ -161,7 +162,7 @@ bool BanMan::Unban(const CNetAddr& net_addr)
 bool BanMan::Unban(const CSubNet& sub_net)
 {
     {
-        LOCK(m_cs_banned);
+        LOCK(m_banned_mutex);
         if (m_banned.erase(sub_net) == 0) return false;
         m_is_dirty = true;
     }
@@ -172,7 +173,7 @@ bool BanMan::Unban(const CSubNet& sub_net)
 
 void BanMan::GetBanned(banmap_t& banmap)
 {
-    LOCK(m_cs_banned);
+    LOCK(m_banned_mutex);
     // Sweep the banlist so expired bans are not returned
     SweepBanned();
     banmap = m_banned; //create a thread safe copy
@@ -180,7 +181,7 @@ void BanMan::GetBanned(banmap_t& banmap)
 
 void BanMan::SweepBanned()
 {
-    AssertLockHeld(m_cs_banned);
+    AssertLockHeld(m_banned_mutex);
 
     int64_t now = GetTime();
     bool notify_ui = false;
@@ -203,15 +204,3 @@ void BanMan::SweepBanned()
         m_client_interface->BannedListChanged();
     }
 }
-
-bool BanMan::BannedSetIsDirty()
-{
-    LOCK(m_cs_banned);
-    return m_is_dirty;
-}
-
-void BanMan::SetBannedSetDirty(bool dirty)
-{
-    LOCK(m_cs_banned); //reuse m_banned lock for the m_is_dirty flag
-    m_is_dirty = dirty;
-}