diff --git a/src/net.cpp b/src/net.cpp index a276f2eaf31..217d9a89037 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1886,7 +1886,7 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ if (max_connections != std::nullopt && existing_connections >= max_connections) return false; // Max total outbound connections already exist - CSemaphoreGrant grant(*semOutbound, true); + CountingSemaphoreGrant<> grant(*semOutbound, true); if (!grant) return false; OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport); @@ -2402,7 +2402,7 @@ void CConnman::ProcessAddrFetch() // peer doesn't support it or immediately disconnects us for another reason. const bool use_v2transport(GetLocalServices() & NODE_P2P_V2); CAddress addr; - CSemaphoreGrant grant(*semOutbound, /*fTry=*/true); + CountingSemaphoreGrant<> grant(*semOutbound, /*fTry=*/true); if (grant) { OpenNetworkConnection(addr, false, std::move(grant), strDest.c_str(), ConnectionType::ADDR_FETCH, use_v2transport); } @@ -2576,7 +2576,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std PerformReconnections(); - CSemaphoreGrant grant(*semOutbound); + CountingSemaphoreGrant<> grant(*semOutbound); if (interruptNet) return; @@ -2954,7 +2954,7 @@ void CConnman::ThreadOpenAddedConnections() AssertLockNotHeld(m_reconnections_mutex); while (true) { - CSemaphoreGrant grant(*semAddnode); + CountingSemaphoreGrant<> grant(*semAddnode); std::vector vInfo = GetAddedNodeInfo(/*include_connected=*/false); bool tried = false; for (const AddedNodeInfo& info : vInfo) { @@ -2967,7 +2967,7 @@ void CConnman::ThreadOpenAddedConnections() CAddress addr(CService(), NODE_NONE); OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport); if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; - grant = CSemaphoreGrant(*semAddnode, /*fTry=*/true); + grant = CountingSemaphoreGrant<>(*semAddnode, /*fTry=*/true); } // See if any reconnections are desired. PerformReconnections(); @@ -2978,7 +2978,7 @@ void CConnman::ThreadOpenAddedConnections() } // if successful, this moves the passed grant to the constructed node -void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char *pszDest, ConnectionType conn_type, bool use_v2transport) +void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CountingSemaphoreGrant<>&& grant_outbound, const char *pszDest, ConnectionType conn_type, bool use_v2transport) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); assert(conn_type != ConnectionType::INBOUND); @@ -3329,11 +3329,11 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) if (semOutbound == nullptr) { // initialize semaphore - semOutbound = std::make_unique(std::min(m_max_automatic_outbound, m_max_automatic_connections)); + semOutbound = std::make_unique>(std::min(m_max_automatic_outbound, m_max_automatic_connections)); } if (semAddnode == nullptr) { // initialize semaphore - semAddnode = std::make_unique(m_max_addnode); + semAddnode = std::make_unique>(m_max_addnode); } // @@ -3421,13 +3421,13 @@ void CConnman::Interrupt() if (semOutbound) { for (int i=0; ipost(); + semOutbound->release(); } } if (semAddnode) { for (int i=0; ipost(); + semAddnode->release(); } } } diff --git a/src/net.h b/src/net.h index 9fdec52115e..4cb4cb906e2 100644 --- a/src/net.h +++ b/src/net.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -729,7 +730,7 @@ public: // Setting fDisconnect to true will cause the node to be disconnected the // next time DisconnectNodes() runs std::atomic_bool fDisconnect{false}; - CSemaphoreGrant grantOutbound; + CountingSemaphoreGrant<> grantOutbound; std::atomic nRefCount{0}; const uint64_t nKeyedNetGroup; @@ -1136,7 +1137,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CountingSemaphoreGrant<>&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); bool CheckIncomingNonce(uint64_t nonce); void ASMapHealthCheck(); @@ -1491,8 +1492,8 @@ private: */ std::atomic m_local_services; - std::unique_ptr semOutbound; - std::unique_ptr semAddnode; + std::unique_ptr> semOutbound; + std::unique_ptr> semAddnode; /** * Maximum number of automatic connections permitted, excluding manual @@ -1614,7 +1615,7 @@ private: struct ReconnectionInfo { CAddress addr_connect; - CSemaphoreGrant grant; + CountingSemaphoreGrant<> grant; std::string destination; ConnectionType conn_type; bool use_v2transport; diff --git a/src/semaphore_grant.h b/src/semaphore_grant.h new file mode 100644 index 00000000000..b1f815f269e --- /dev/null +++ b/src/semaphore_grant.h @@ -0,0 +1,93 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_SEMAPHORE_GRANT_H +#define BITCOIN_SEMAPHORE_GRANT_H + +#include + +/** RAII-style semaphore lock */ +template ::max()> +class CountingSemaphoreGrant +{ +private: + std::counting_semaphore* sem; + bool fHaveGrant; + +public: + void Acquire() noexcept + { + if (fHaveGrant) { + return; + } + sem->acquire(); + fHaveGrant = true; + } + + void Release() noexcept + { + if (!fHaveGrant) { + return; + } + sem->release(); + fHaveGrant = false; + } + + bool TryAcquire() noexcept + { + if (!fHaveGrant && sem->try_acquire()) { + fHaveGrant = true; + } + return fHaveGrant; + } + + // Disallow copy. + CountingSemaphoreGrant(const CountingSemaphoreGrant&) = delete; + CountingSemaphoreGrant& operator=(const CountingSemaphoreGrant&) = delete; + + // Allow move. + CountingSemaphoreGrant(CountingSemaphoreGrant&& other) noexcept + { + sem = other.sem; + fHaveGrant = other.fHaveGrant; + other.fHaveGrant = false; + other.sem = nullptr; + } + + CountingSemaphoreGrant& operator=(CountingSemaphoreGrant&& other) noexcept + { + Release(); + sem = other.sem; + fHaveGrant = other.fHaveGrant; + other.fHaveGrant = false; + other.sem = nullptr; + return *this; + } + + CountingSemaphoreGrant() noexcept : sem(nullptr), fHaveGrant(false) {} + + explicit CountingSemaphoreGrant(std::counting_semaphore& sema, bool fTry = false) noexcept : sem(&sema), fHaveGrant(false) + { + if (fTry) { + TryAcquire(); + } else { + Acquire(); + } + } + + ~CountingSemaphoreGrant() + { + Release(); + } + + explicit operator bool() const noexcept + { + return fHaveGrant; + } +}; + +using BinarySemaphoreGrant = CountingSemaphoreGrant<1>; + +#endif // BITCOIN_SEMAPHORE_GRANT_H diff --git a/src/sync.h b/src/sync.h index b22956ef1ab..6eb45f29884 100644 --- a/src/sync.h +++ b/src/sync.h @@ -300,131 +300,4 @@ inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNE //! gcc and the -Wreturn-stack-address flag in clang, both enabled by default. #define WITH_LOCK(cs, code) (MaybeCheckNotHeld(cs), [&]() -> decltype(auto) { LOCK(cs); code; }()) -/** An implementation of a semaphore. - * - * See https://en.wikipedia.org/wiki/Semaphore_(programming) - */ -class CSemaphore -{ -private: - std::condition_variable condition; - std::mutex mutex; - int value; - -public: - explicit CSemaphore(int init) noexcept : value(init) {} - - // Disallow default construct, copy, move. - CSemaphore() = delete; - CSemaphore(const CSemaphore&) = delete; - CSemaphore(CSemaphore&&) = delete; - CSemaphore& operator=(const CSemaphore&) = delete; - CSemaphore& operator=(CSemaphore&&) = delete; - - void wait() noexcept - { - std::unique_lock lock(mutex); - condition.wait(lock, [&]() { return value >= 1; }); - value--; - } - - bool try_wait() noexcept - { - std::lock_guard lock(mutex); - if (value < 1) { - return false; - } - value--; - return true; - } - - void post() noexcept - { - { - std::lock_guard lock(mutex); - value++; - } - condition.notify_one(); - } -}; - -/** RAII-style semaphore lock */ -class CSemaphoreGrant -{ -private: - CSemaphore* sem; - bool fHaveGrant; - -public: - void Acquire() noexcept - { - if (fHaveGrant) { - return; - } - sem->wait(); - fHaveGrant = true; - } - - void Release() noexcept - { - if (!fHaveGrant) { - return; - } - sem->post(); - fHaveGrant = false; - } - - bool TryAcquire() noexcept - { - if (!fHaveGrant && sem->try_wait()) { - fHaveGrant = true; - } - return fHaveGrant; - } - - // Disallow copy. - CSemaphoreGrant(const CSemaphoreGrant&) = delete; - CSemaphoreGrant& operator=(const CSemaphoreGrant&) = delete; - - // Allow move. - CSemaphoreGrant(CSemaphoreGrant&& other) noexcept - { - sem = other.sem; - fHaveGrant = other.fHaveGrant; - other.fHaveGrant = false; - other.sem = nullptr; - } - - CSemaphoreGrant& operator=(CSemaphoreGrant&& other) noexcept - { - Release(); - sem = other.sem; - fHaveGrant = other.fHaveGrant; - other.fHaveGrant = false; - other.sem = nullptr; - return *this; - } - - CSemaphoreGrant() noexcept : sem(nullptr), fHaveGrant(false) {} - - explicit CSemaphoreGrant(CSemaphore& sema, bool fTry = false) noexcept : sem(&sema), fHaveGrant(false) - { - if (fTry) { - TryAcquire(); - } else { - Acquire(); - } - } - - ~CSemaphoreGrant() - { - Release(); - } - - explicit operator bool() const noexcept - { - return fHaveGrant; - } -}; - #endif // BITCOIN_SYNC_H diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 49096c91aa3..9b6215f9371 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -445,7 +445,7 @@ void SQLiteBatch::Close() try { m_database.Open(); // If TxnAbort failed and we refreshed the connection, the semaphore was not released, so release it here to avoid deadlocks on future writes. - m_database.m_write_semaphore.post(); + m_database.m_write_semaphore.release(); } catch (const std::runtime_error&) { // If open fails, cleanup this object and rethrow the exception m_database.Close(); @@ -498,7 +498,7 @@ bool SQLiteBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite) if (!BindBlobToStatement(stmt, 2, value, "value")) return false; // Acquire semaphore if not previously acquired when creating a transaction. - if (!m_txn) m_database.m_write_semaphore.wait(); + if (!m_txn) m_database.m_write_semaphore.acquire(); // Execute int res = sqlite3_step(stmt); @@ -508,7 +508,7 @@ bool SQLiteBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite) LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res)); } - if (!m_txn) m_database.m_write_semaphore.post(); + if (!m_txn) m_database.m_write_semaphore.release(); return res == SQLITE_DONE; } @@ -522,7 +522,7 @@ bool SQLiteBatch::ExecStatement(sqlite3_stmt* stmt, std::span b if (!BindBlobToStatement(stmt, 1, blob, "key")) return false; // Acquire semaphore if not previously acquired when creating a transaction. - if (!m_txn) m_database.m_write_semaphore.wait(); + if (!m_txn) m_database.m_write_semaphore.acquire(); // Execute int res = sqlite3_step(stmt); @@ -532,7 +532,7 @@ bool SQLiteBatch::ExecStatement(sqlite3_stmt* stmt, std::span b LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res)); } - if (!m_txn) m_database.m_write_semaphore.post(); + if (!m_txn) m_database.m_write_semaphore.release(); return res == SQLITE_DONE; } @@ -651,12 +651,12 @@ std::unique_ptr SQLiteBatch::GetNewPrefixCursor(std::spanExec(m_database, "BEGIN TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to begin the transaction\n"); - m_database.m_write_semaphore.post(); + m_database.m_write_semaphore.release(); } else { m_txn = true; } @@ -672,7 +672,7 @@ bool SQLiteBatch::TxnCommit() LogPrintf("SQLiteBatch: Failed to commit the transaction\n"); } else { m_txn = false; - m_database.m_write_semaphore.post(); + m_database.m_write_semaphore.release(); } return res == SQLITE_OK; } @@ -686,7 +686,7 @@ bool SQLiteBatch::TxnAbort() LogPrintf("SQLiteBatch: Failed to abort the transaction\n"); } else { m_txn = false; - m_database.m_write_semaphore.post(); + m_database.m_write_semaphore.release(); } return res == SQLITE_OK; } diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index a3e0de1fcc2..14ad38792c4 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -8,6 +8,8 @@ #include #include +#include + struct bilingual_str; struct sqlite3_stmt; @@ -127,7 +129,7 @@ public: // Batches must acquire this semaphore on writing, and release when done writing. // This ensures that only one batch is modifying the database at a time. - CSemaphore m_write_semaphore; + std::binary_semaphore m_write_semaphore; bool Verify(bilingual_str& error);