highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-10-11 03:53:22 +02:00
Code Issues Packages Projects Releases Wiki Activity

net: Add interrupt to pcp retry loop

Browse Source 
Without this interrupt bitcoind takes a long time to exit if requested
to do so after a failed pcp lookup on startup.
This commit is contained in:
TheCharlatan
2025-09-08 11:13:52 +02:00
parent e1ce0c525c
commit 188de70c86
5 changed files with 36 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/common/pcp.cpp
View File

@@ -15,6 +15,7 @@
#include <util/readwritefile.h> #include <util/readwritefile.h>
#include <util/sock.h> #include <util/sock.h>
#include <util/strencodings.h> #include <util/strencodings.h>
#include <util/threadinterrupt.h>
namespace { namespace {
@@ -217,7 +218,8 @@ CNetAddr PCPUnwrapAddress(std::span<const uint8_t> wrapped_addr)
//! PCP or NAT-PMP send-receive loop. //! PCP or NAT-PMP send-receive loop.
std::optional<std::vector<uint8_t>> PCPSendRecv(Sock &sock, const std::string &protocol, std::span<const uint8_t> request, int num_tries, std::optional<std::vector<uint8_t>> PCPSendRecv(Sock &sock, const std::string &protocol, std::span<const uint8_t> request, int num_tries,
std::chrono::milliseconds timeout_per_try, std::chrono::milliseconds timeout_per_try,
std::function<bool(std::span<const uint8_t>)> check_packet) std::function<bool(std::span<const uint8_t>)> check_packet,
CThreadInterrupt& interrupt)
{ {
using namespace std::chrono; using namespace std::chrono;
// UDP is a potentially lossy protocol, so we try to send again a few times. // UDP is a potentially lossy protocol, so we try to send again a few times.
@@ -238,6 +240,7 @@ std::optional<std::vector<uint8_t>> PCPSendRecv(Sock &sock, const std::string &p
auto cur_time = time_point_cast<milliseconds>(MockableSteadyClock::now()); auto cur_time = time_point_cast<milliseconds>(MockableSteadyClock::now());
auto deadline = cur_time + timeout_per_try; auto deadline = cur_time + timeout_per_try;
while ((cur_time = time_point_cast<milliseconds>(MockableSteadyClock::now())) < deadline) { while ((cur_time = time_point_cast<milliseconds>(MockableSteadyClock::now())) < deadline) {
if (interrupt) return std::nullopt;
Sock::Event occurred = 0; Sock::Event occurred = 0;
if (!sock.Wait(deadline - cur_time, Sock::RECV, &occurred)) { if (!sock.Wait(deadline - cur_time, Sock::RECV, &occurred)) {
LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s: Could not wait on socket: %s\n", protocol, NetworkErrorString(WSAGetLastError())); LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s: Could not wait on socket: %s\n", protocol, NetworkErrorString(WSAGetLastError()));
@@ -271,7 +274,7 @@ std::optional<std::vector<uint8_t>> PCPSendRecv(Sock &sock, const std::string &p
} }
std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, int num_tries, std::chrono::milliseconds timeout_per_try) std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, CThreadInterrupt& interrupt, int num_tries, std::chrono::milliseconds timeout_per_try)
{ {
struct sockaddr_storage dest_addr; struct sockaddr_storage dest_addr;
socklen_t dest_addrlen = sizeof(struct sockaddr_storage); socklen_t dest_addrlen = sizeof(struct sockaddr_storage);
@@ -319,7 +322,8 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
return false; // Wasn't response to what we expected, try receiving next packet. return false; // Wasn't response to what we expected, try receiving next packet.
} }
return true; return true;
}); },
interrupt);
struct in_addr external_addr; struct in_addr external_addr;
if (recv_res) { if (recv_res) {
@@ -361,7 +365,8 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
return false; // Wasn't response to what we expected, try receiving next packet. return false; // Wasn't response to what we expected, try receiving next packet.
} }
return true; return true;
}); },
interrupt);
if (recv_res) { if (recv_res) {
const std::span<uint8_t> response = *recv_res; const std::span<uint8_t> response = *recv_res;
@@ -384,7 +389,7 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
} }
} }
std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, int num_tries, std::chrono::milliseconds timeout_per_try) std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, CThreadInterrupt& interrupt, int num_tries, std::chrono::milliseconds timeout_per_try)
{ {
struct sockaddr_storage dest_addr, bind_addr; struct sockaddr_storage dest_addr, bind_addr;
socklen_t dest_addrlen = sizeof(struct sockaddr_storage), bind_addrlen = sizeof(struct sockaddr_storage); socklen_t dest_addrlen = sizeof(struct sockaddr_storage), bind_addrlen = sizeof(struct sockaddr_storage);
@@ -484,7 +489,8 @@ std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonc
return false; // Wasn't response to what we expected, try receiving next packet. return false; // Wasn't response to what we expected, try receiving next packet.
} }
return true; return true;
}); },
interrupt);
if (!recv_res) { if (!recv_res) {
return MappingError::NETWORK_ERROR; return MappingError::NETWORK_ERROR;

5
src/common/pcp.h
View File

@@ -6,6 +6,7 @@
#define BITCOIN_COMMON_PCP_H #define BITCOIN_COMMON_PCP_H
#include <netaddress.h> #include <netaddress.h>
#include <util/threadinterrupt.h>
#include <variant> #include <variant>
@@ -51,7 +52,7 @@ struct MappingResult {
//! * num_tries: Number of tries in case of no response. //! * num_tries: Number of tries in case of no response.
//! //!
//! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError. //! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError.
std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000)); std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, CThreadInterrupt& interrupt, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000));
//! Try to open a port using RFC 6887 Port Control Protocol (PCP). Handles IPv4 and IPv6. //! Try to open a port using RFC 6887 Port Control Protocol (PCP). Handles IPv4 and IPv6.
//! //!
@@ -63,6 +64,6 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
//! * num_tries: Number of tries in case of no response. //! * num_tries: Number of tries in case of no response.
//! //!
//! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError. //! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError.
std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000)); std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, CThreadInterrupt& interrupt, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000));
#endif // BITCOIN_COMMON_PCP_H #endif // BITCOIN_COMMON_PCP_H

6
src/mapport.cpp
View File

@@ -74,11 +74,11 @@ static void ProcessPCP()
// Open a port mapping on whatever local address we have toward the gateway. // Open a port mapping on whatever local address we have toward the gateway.
struct in_addr inaddr_any; struct in_addr inaddr_any;
inaddr_any.s_addr = htonl(INADDR_ANY); inaddr_any.s_addr = htonl(INADDR_ANY);
auto res = PCPRequestPortMap(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime); auto res = PCPRequestPortMap(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime, g_mapport_interrupt);
MappingError* pcp_err = std::get_if<MappingError>(&res); MappingError* pcp_err = std::get_if<MappingError>(&res);
if (pcp_err && *pcp_err == MappingError::UNSUPP_VERSION) { if (pcp_err && *pcp_err == MappingError::UNSUPP_VERSION) {
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Got unsupported PCP version response, falling back to NAT-PMP\n"); LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Got unsupported PCP version response, falling back to NAT-PMP\n");
res = NATPMPRequestPortMap(*gateway4, private_port, requested_lifetime); res = NATPMPRequestPortMap(*gateway4, private_port, requested_lifetime, g_mapport_interrupt);
} }
handle_mapping(res); handle_mapping(res);
} }
@@ -93,7 +93,7 @@ static void ProcessPCP()
// Try to open pinholes for all routable local IPv6 addresses. // Try to open pinholes for all routable local IPv6 addresses.
for (const auto &addr: GetLocalAddresses()) { for (const auto &addr: GetLocalAddresses()) {
if (!addr.IsRoutable() || !addr.IsIPv6()) continue; if (!addr.IsRoutable() || !addr.IsIPv6()) continue;
auto res = PCPRequestPortMap(pcp_nonce, *gateway6, addr, private_port, requested_lifetime); auto res = PCPRequestPortMap(pcp_nonce, *gateway6, addr, private_port, requested_lifetime, g_mapport_interrupt);
handle_mapping(res); handle_mapping(res);
} }
} }

7
src/test/fuzz/pcp.cpp
View File

@@ -9,6 +9,7 @@
#include <common/pcp.h> #include <common/pcp.h>
#include <util/check.h> #include <util/check.h>
#include <util/threadinterrupt.h>
using namespace std::literals; using namespace std::literals;
@@ -43,7 +44,8 @@ FUZZ_TARGET(pcp_request_port_map, .init = port_map_target_init)
const auto local_addr{ConsumeNetAddr(fuzzed_data_provider)}; const auto local_addr{ConsumeNetAddr(fuzzed_data_provider)};
const auto port{fuzzed_data_provider.ConsumeIntegral<uint16_t>()}; const auto port{fuzzed_data_provider.ConsumeIntegral<uint16_t>()};
const auto lifetime{fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; const auto lifetime{fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
const auto res{PCPRequestPortMap(PCP_NONCE, gateway_addr, local_addr, port, lifetime, NUM_TRIES, TIMEOUT)}; CThreadInterrupt interrupt;
const auto res{PCPRequestPortMap(PCP_NONCE, gateway_addr, local_addr, port, lifetime, interrupt, NUM_TRIES, TIMEOUT)};
// In case of success the mapping must be consistent with the request. // In case of success the mapping must be consistent with the request.
if (const MappingResult* mapping = std::get_if<MappingResult>(&res)) { if (const MappingResult* mapping = std::get_if<MappingResult>(&res)) {
@@ -69,7 +71,8 @@ FUZZ_TARGET(natpmp_request_port_map, .init = port_map_target_init)
const auto gateway_addr{ConsumeNetAddr(fuzzed_data_provider)}; const auto gateway_addr{ConsumeNetAddr(fuzzed_data_provider)};
const auto port{fuzzed_data_provider.ConsumeIntegral<uint16_t>()}; const auto port{fuzzed_data_provider.ConsumeIntegral<uint16_t>()};
const auto lifetime{fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; const auto lifetime{fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
const auto res{NATPMPRequestPortMap(gateway_addr, port, lifetime, NUM_TRIES, TIMEOUT)}; CThreadInterrupt interrupt;
const auto res{NATPMPRequestPortMap(gateway_addr, port, lifetime, interrupt, NUM_TRIES, TIMEOUT)};
// In case of success the mapping must be consistent with the request. // In case of success the mapping must be consistent with the request.
if (const MappingResult* mapping = std::get_if<MappingResult>(&res)) { if (const MappingResult* mapping = std::get_if<MappingResult>(&res)) {

24
src/test/pcp_tests.cpp
View File

@@ -15,6 +15,8 @@
using namespace std::literals; using namespace std::literals;
static CThreadInterrupt g_interrupt;
/// UDP test server operation. /// UDP test server operation.
struct TestOp { struct TestOp {
std::chrono::milliseconds delay; std::chrono::milliseconds delay;
@@ -295,7 +297,7 @@ BOOST_AUTO_TEST_CASE(natpmp_ipv4)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, 1, 200ms); auto res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, g_interrupt, 1, 200ms);
MappingResult* mapping = std::get_if<MappingResult>(&res); MappingResult* mapping = std::get_if<MappingResult>(&res);
BOOST_REQUIRE(mapping); BOOST_REQUIRE(mapping);
@@ -339,7 +341,7 @@ BOOST_AUTO_TEST_CASE(pcp_ipv4)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 1, 1000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 1, 1000ms);
MappingResult* mapping = std::get_if<MappingResult>(&res); MappingResult* mapping = std::get_if<MappingResult>(&res);
BOOST_REQUIRE(mapping); BOOST_REQUIRE(mapping);
@@ -383,7 +385,7 @@ BOOST_AUTO_TEST_CASE(pcp_ipv6)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv6, default_local_ipv6, 1234, 1000, 1, 1000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv6, default_local_ipv6, 1234, 1000, g_interrupt, 1, 1000ms);
MappingResult* mapping = std::get_if<MappingResult>(&res); MappingResult* mapping = std::get_if<MappingResult>(&res);
BOOST_REQUIRE(mapping); BOOST_REQUIRE(mapping);
@@ -406,7 +408,7 @@ BOOST_AUTO_TEST_CASE(pcp_timeout)
ASSERT_DEBUG_LOG("pcp: Retrying (2)"); ASSERT_DEBUG_LOG("pcp: Retrying (2)");
ASSERT_DEBUG_LOG("pcp: Timeout"); ASSERT_DEBUG_LOG("pcp: Timeout");
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 3, 2000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 3, 2000ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -435,7 +437,7 @@ BOOST_AUTO_TEST_CASE(pcp_connrefused)
ASSERT_DEBUG_LOG("pcp: Could not receive response"); ASSERT_DEBUG_LOG("pcp: Could not receive response");
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 3, 2000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 3, 2000ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -495,7 +497,7 @@ BOOST_AUTO_TEST_CASE(pcp_ipv6_timeout_success)
ASSERT_DEBUG_LOG("pcp: Retrying (1)"); ASSERT_DEBUG_LOG("pcp: Retrying (1)");
ASSERT_DEBUG_LOG("pcp: Timeout"); ASSERT_DEBUG_LOG("pcp: Timeout");
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv6, default_local_ipv6, 1234, 1000, 2, 2000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv6, default_local_ipv6, 1234, 1000, g_interrupt, 2, 2000ms);
BOOST_CHECK(std::get_if<MappingResult>(&res)); BOOST_CHECK(std::get_if<MappingResult>(&res));
} }
@@ -534,7 +536,7 @@ BOOST_AUTO_TEST_CASE(pcp_ipv4_fail_no_resources)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 3, 1000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 3, 1000ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -570,7 +572,7 @@ BOOST_AUTO_TEST_CASE(pcp_ipv4_fail_unsupported_version)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 3, 1000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 3, 1000ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -602,7 +604,7 @@ BOOST_AUTO_TEST_CASE(natpmp_protocol_error)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, 1, 200ms); auto res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, g_interrupt, 1, 200ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -647,7 +649,7 @@ BOOST_AUTO_TEST_CASE(natpmp_protocol_error)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, 1, 200ms); res = NATPMPRequestPortMap(default_gateway_ipv4, 1234, 1000, g_interrupt, 1, 200ms);
err = std::get_if<MappingError>(&res); err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);
@@ -688,7 +690,7 @@ BOOST_AUTO_TEST_CASE(pcp_protocol_error)
return std::unique_ptr<PCPTestSock>(); return std::unique_ptr<PCPTestSock>();
}; };
auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, 1, 1000ms); auto res = PCPRequestPortMap(TEST_NONCE, default_gateway_ipv4, bind_any_ipv4, 1234, 1000, g_interrupt, 1, 1000ms);
MappingError* err = std::get_if<MappingError>(&res); MappingError* err = std::get_if<MappingError>(&res);
BOOST_REQUIRE(err); BOOST_REQUIRE(err);