Merge bitcoin/bitcoin#33338: net: Add interrupt to pcp retry loop

188de70c86 net: Add interrupt to pcp retry loop (TheCharlatan)

Pull request description:

  Without this interrupt bitcoind takes a long time to exit if requested to do so after a failed pcp lookup on startup.

ACKs for top commit:
  achow101:
    ACK 188de70c86
  fjahr:
    utACK 188de70c86
  hodlinator:
    utACK 188de70c86

Tree-SHA512: 426dabd10ac0ef5de246c83d281ba70957e4032d251054aa6028b4d7ce4e35cd35ac70e67dc07bd418673bcdd2f4457b76f174ac5e7d0dd3caa05de5da952dac
This commit is contained in:
Ava Chow
2025-09-08 13:44:44 -07:00
5 changed files with 36 additions and 24 deletions

View File

@@ -15,6 +15,7 @@
#include <util/readwritefile.h>
#include <util/sock.h>
#include <util/strencodings.h>
#include <util/threadinterrupt.h>
namespace {
@@ -217,7 +218,8 @@ CNetAddr PCPUnwrapAddress(std::span<const uint8_t> wrapped_addr)
//! 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::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;
// 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 deadline = cur_time + timeout_per_try;
while ((cur_time = time_point_cast<milliseconds>(MockableSteadyClock::now())) < deadline) {
if (interrupt) return std::nullopt;
Sock::Event occurred = 0;
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()));
@@ -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;
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 true;
});
},
interrupt);
struct in_addr external_addr;
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 true;
});
},
interrupt);
if (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;
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 true;
});
},
interrupt);
if (!recv_res) {
return MappingError::NETWORK_ERROR;

View File

@@ -6,6 +6,7 @@
#define BITCOIN_COMMON_PCP_H
#include <netaddress.h>
#include <util/threadinterrupt.h>
#include <variant>
@@ -51,7 +52,7 @@ struct MappingResult {
//! * num_tries: Number of tries in case of no response.
//!
//! 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.
//!
@@ -63,6 +64,6 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
//! * num_tries: Number of tries in case of no response.
//!
//! 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