diff --git a/src/i2p.cpp b/src/i2p.cpp index 23f93a2ad7275..f189350fd881d 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -444,7 +444,7 @@ void Session::Disconnect() Log("Destroying SAM session %s", m_session_id); } } - m_control_sock->Reset(); + m_control_sock = std::make_unique(INVALID_SOCKET); m_session_id.clear(); } } // namespace sam diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 6f159ee1586a2..e2daa96bf71df 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -163,7 +163,7 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex) return; } bool fConnected = ConnectSocketDirectly(m_info.service, *sock, nConnectTimeout, true) && sock->IsSelectable(); - sock->Reset(); + sock = std::make_unique(INVALID_SOCKET); if (!fConnected && Params().RequireRoutableExternalIP()) { m_state = MasternodeState::SOME_ERROR; diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 4bc84f40cdb9c..8ffa0ace84967 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -22,10 +22,10 @@ FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) FuzzedSock::~FuzzedSock() { // Sock::~Sock() will be called after FuzzedSock::~FuzzedSock() and it will call - // Sock::Reset() (not FuzzedSock::Reset()!) which will call CloseSocket(m_socket). + // close(m_socket) if m_socket is not INVALID_SOCKET. // Avoid closing an arbitrary file descriptor (m_socket is just a random very high number which // theoretically may concide with a real opened file descriptor). - Reset(); + m_socket = INVALID_SOCKET; } FuzzedSock& FuzzedSock::operator=(Sock&& other) @@ -34,11 +34,6 @@ FuzzedSock& FuzzedSock::operator=(Sock&& other) return *this; } -void FuzzedSock::Reset() -{ - m_socket = INVALID_SOCKET; -} - ssize_t FuzzedSock::Send(const void* data, size_t len, int flags) const { constexpr std::array send_errnos{ diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 67296f00c24ee..9da9c9f42abc2 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -67,8 +67,6 @@ class FuzzedSock : public Sock FuzzedSock& operator=(Sock&& other) override; - void Reset() override; - ssize_t Send(const void* data, size_t len, int flags) const override; ssize_t Recv(void* buf, size_t len, int flags) const override; diff --git a/src/test/sock_tests.cpp b/src/test/sock_tests.cpp index fa95fc0ba55ea..8376ec1a68471 100644 --- a/src/test/sock_tests.cpp +++ b/src/test/sock_tests.cpp @@ -69,24 +69,6 @@ BOOST_AUTO_TEST_CASE(move_assignment) BOOST_CHECK(SocketIsClosed(s)); } -BOOST_AUTO_TEST_CASE(release) -{ - SOCKET s = CreateSocket(); - Sock* sock = new Sock(s); - BOOST_CHECK_EQUAL(sock->Release(), s); - delete sock; - BOOST_CHECK(!SocketIsClosed(s)); - BOOST_REQUIRE(CloseSocket(s)); -} - -BOOST_AUTO_TEST_CASE(reset) -{ - const SOCKET s = CreateSocket(); - Sock sock(s); - sock.Reset(); - BOOST_CHECK(SocketIsClosed(s)); -} - #ifndef WIN32 // Windows does not have socketpair(2). static void CreateSocketPair(int s[2]) diff --git a/src/test/util/net.h b/src/test/util/net.h index 64d3811107be6..c647c11a8d95b 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -139,7 +139,7 @@ class StaticContentsSock : public Sock m_socket = INVALID_SOCKET - 1; } - ~StaticContentsSock() override { Reset(); } + ~StaticContentsSock() override { m_socket = INVALID_SOCKET; } StaticContentsSock& operator=(Sock&& other) override { @@ -147,11 +147,6 @@ class StaticContentsSock : public Sock return *this; } - void Reset() override - { - m_socket = INVALID_SOCKET; - } - ssize_t Send(const void*, size_t len, int) const override { return len; } ssize_t Recv(void* buf, size_t len, int flags) const override diff --git a/src/util/sock.cpp b/src/util/sock.cpp index 423a241673cba..4913e31cd28c1 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -39,11 +39,11 @@ Sock::Sock(Sock&& other) other.m_socket = INVALID_SOCKET; } -Sock::~Sock() { Reset(); } +Sock::~Sock() { Close(); } Sock& Sock::operator=(Sock&& other) { - Reset(); + Close(); m_socket = other.m_socket; other.m_socket = INVALID_SOCKET; return *this; @@ -51,15 +51,6 @@ Sock& Sock::operator=(Sock&& other) SOCKET Sock::Get() const { return m_socket; } -SOCKET Sock::Release() -{ - const SOCKET s = m_socket; - m_socket = INVALID_SOCKET; - return s; -} - -void Sock::Reset() { CloseSocket(m_socket); } - ssize_t Sock::Send(const void* data, size_t len, int flags) const { return send(m_socket, static_cast(data), len, flags); @@ -369,6 +360,22 @@ bool Sock::IsConnected(std::string& errmsg) const } } +void Sock::Close() +{ + if (m_socket == INVALID_SOCKET) { + return; + } +#ifdef WIN32 + int ret = closesocket(m_socket); +#else + int ret = close(m_socket); +#endif + if (ret) { + LogPrintf("Error closing socket %d: %s\n", m_socket, NetworkErrorString(WSAGetLastError())); + } + m_socket = INVALID_SOCKET; +} + #ifdef WIN32 std::string NetworkErrorString(int err) { @@ -392,19 +399,3 @@ std::string NetworkErrorString(int err) return SysErrorString(err); } #endif - -bool CloseSocket(SOCKET& hSocket) -{ - if (hSocket == INVALID_SOCKET) - return false; -#ifdef WIN32 - int ret = closesocket(hSocket); -#else - int ret = close(hSocket); -#endif - if (ret) { - LogPrintf("Socket close failed: %d. Error: %s\n", hSocket, NetworkErrorString(WSAGetLastError())); - } - hSocket = INVALID_SOCKET; - return ret != SOCKET_ERROR; -} diff --git a/src/util/sock.h b/src/util/sock.h index 2edd79847b968..b1775c3c929cf 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -113,18 +113,6 @@ class Sock */ [[nodiscard]] virtual SOCKET Get() const; - /** - * Get the value of the contained socket and drop ownership. It will not be closed by the - * destructor after this call. - * @return socket or INVALID_SOCKET if empty - */ - virtual SOCKET Release(); - - /** - * Close if non-empty. - */ - virtual void Reset(); - /** * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. @@ -270,12 +258,15 @@ class Sock * Contained socket. `INVALID_SOCKET` designates the object is empty. */ SOCKET m_socket; + +private: + /** + * Close `m_socket` if it is not `INVALID_SOCKET`. + */ + void Close(); }; /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); -/** Close socket and set hSocket to INVALID_SOCKET */ -bool CloseSocket(SOCKET& hSocket); - #endif // BITCOIN_UTIL_SOCK_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 358010c7ce4e3..754058f33b27c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -643,16 +643,13 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid, Walle } -void CWallet::AddToSpends(const uint256& wtxid, WalletBatch* batch) +void CWallet::AddToSpends(const CWalletTx& wtx, WalletBatch* batch) { - auto it = mapWallet.find(wtxid); - assert(it != mapWallet.end()); - const CWalletTx& thisTx = it->second; - if (thisTx.IsCoinBase()) // Coinbases don't spend anything! + if (wtx.IsCoinBase()) // Coinbases don't spend anything! return; - for (const CTxIn& txin : thisTx.tx->vin) - AddToSpends(txin.prevout, wtxid, batch); + for (const CTxIn& txin : wtx.tx->vin) + AddToSpends(txin.prevout, wtx.GetHash(), batch); } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) @@ -921,7 +918,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); - AddToSpends(hash, &batch); + AddToSpends(wtx, &batch); candidates = AddWalletUTXOs(wtx.tx, /*ret_dups=*/true); } @@ -1024,7 +1021,7 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx if (/* insertion took place */ ins.second) { wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); } - AddToSpends(hash); + AddToSpends(wtx); for (const CTxIn& txin : wtx.tx->vin) { auto it = mapWallet.find(txin.prevout.hash); if (it != mapWallet.end()) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5a36f9d662bf9..5a9c1d606514c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -760,7 +760,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati typedef std::multimap TxSpends; TxSpends mapTxSpends GUARDED_BY(cs_wallet); void AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void AddToSpends(const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set setWalletUTXO; /** Add new UTXOs to the wallet UTXO set