timeout_duration is passed into HTTPClient::Connect(host, port, timeout_duration), but it is only stored in HTTPClient::m_timeout after ConnectDirectly() has already returned a connected socket.
It is then used for HTTP send/read, not for the TCP connect attempt itself, which happens in auto sock = ConnectDirectly(service, /*manual_connection=*/true); and uses nConnectTimeout.
In the old libevent code, evhttp_connection_set_timeout() attached the -rpcclienttimeout value to the libevent HTTP connection before evhttp_make_request() started the connection/request, so the timeout also covered the TCP connect phase.
Suggested diff to preserve the previous behavior (considering that is the intention):
<details>
<summary>diff</summary>
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 5cdf668164..0fa2272989 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -871,8 +871,13 @@ HTTPClient HTTPClient::Connect(const std::string& host, uint16_t port, std::chro
throw CConnectionFailed(strprintf("Could not resolve host: %s", host));
}
+ const auto deadline{std::chrono::steady_clock::now() + timeout};
for (const CService& service : services) {
- auto sock = ConnectDirectly(service, /*manual_connection=*/true);
+ const auto now{std::chrono::steady_clock::now()};
+ const auto time_left{std::chrono::duration_cast<std::chrono::milliseconds>(deadline - now)};
+ if (time_left.count() <= 0) break;
+
+ auto sock = ConnectDirectly(service, /*manual_connection=*/true, time_left);
if (sock) return HTTPClient{std::move(sock), host, timeout};
}
diff --git a/src/netbase.cpp b/src/netbase.cpp
index 5434ec9f14..65d8c28d63 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -587,7 +587,12 @@ static void LogConnectFailure(bool manual_connection, util::ConstevalFormatStrin
}
}
-static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen_t len, const std::string& dest_str, bool manual_connection)
+static bool ConnectToSocket(const Sock& sock,
+ struct sockaddr* sockaddr,
+ socklen_t len,
+ const std::string& dest_str,
+ bool manual_connection,
+ std::chrono::milliseconds timeout)
{
// Connect to `sockaddr` using `sock`.
if (sock.Connect(sockaddr, len) == SOCKET_ERROR) {
@@ -600,7 +605,7 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen
// synchronously to check for successful connection with a timeout.
const Sock::Event requested = Sock::RECV | Sock::SEND;
Sock::Event occurred;
- if (!sock.Wait(std::chrono::milliseconds{nConnectTimeout}, requested, &occurred)) {
+ if (!sock.Wait(timeout, requested, &occurred)) {
LogInfo("wait for connect to %s failed: %s\n",
dest_str,
NetworkErrorString(WSAGetLastError()));
@@ -643,6 +648,13 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen
}
std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connection)
+{
+ return ConnectDirectly(dest, manual_connection, std::chrono::milliseconds{nConnectTimeout});
+}
+
+std::unique_ptr<Sock> ConnectDirectly(const CService& dest,
+ bool manual_connection,
+ std::chrono::milliseconds timeout)
{
auto sock = CreateSock(dest.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP);
if (!sock) {
@@ -658,7 +670,7 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti
return {};
}
- if (!ConnectToSocket(*sock, (struct sockaddr*)&sockaddr, len, dest.ToStringAddrPort(), manual_connection)) {
+ if (!ConnectToSocket(*sock, (struct sockaddr*)&sockaddr, len, dest.ToStringAddrPort(), manual_connection, timeout)) {
return {};
}
@@ -687,7 +699,12 @@ std::unique_ptr<Sock> Proxy::Connect() const
memcpy(addrun.sun_path, path.c_str(), std::min(sizeof(addrun.sun_path) - 1, path.length()));
socklen_t len = sizeof(addrun);
- if(!ConnectToSocket(*sock, (struct sockaddr*)&addrun, len, path, /*manual_connection=*/true)) {
+ if (!ConnectToSocket(*sock,
+ (struct sockaddr*)&addrun,
+ len,
+ path,
+ /*manual_connection=*/true,
+ std::chrono::milliseconds{nConnectTimeout})) {
return {};
}
diff --git a/src/netbase.h b/src/netbase.h
index 88cc69810b..af51853a51 100644
--- a/src/netbase.h
+++ b/src/netbase.h
@@ -11,6 +11,7 @@
#include <util/sock.h>
#include <util/threadinterrupt.h>
+#include <chrono>
#include <cstdint>
#include <functional>
#include <memory>
@@ -305,6 +306,11 @@ extern std::function<std::unique_ptr<Sock>(int, int, int)> CreateSock;
*/
std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connection);
+/** Create a socket and try to connect to the specified service, using the provided timeout. */
+std::unique_ptr<Sock> ConnectDirectly(const CService& dest,
+ bool manual_connection,
+ std::chrono::milliseconds timeout);
+
/**
* Connect to a specified destination service through a SOCKS5 proxy by first
* connecting to the SOCKS5 proxy.
</details>
