diff --git a/include/miniocpp/http.h b/include/miniocpp/http.h index 4596ed08..c884087d 100644 --- a/include/miniocpp/http.h +++ b/include/miniocpp/http.h @@ -130,6 +130,12 @@ struct Request { // defaults. Used by the RDMA control plane to fail fast on a dead NIC so // the caller can retry (and pick up the failover NIC on the next attempt) // instead of blocking on TCP's default ~75s SYN timeout. + // + // Note: leaving timeout_secs == 0 still installs a low-speed stall guard + // (abort if throughput stays below 1 byte/s for 60s) so a dropped/stalled + // connection can't hang the transfer forever. It does not bound a healthy + // transfer's total duration. Set timeout_secs > 0 for a hard total timeout + // (which replaces the stall guard). long connect_timeout_secs = 0; long timeout_secs = 0; diff --git a/src/http.cc b/src/http.cc index 572803aa..30f7f998 100644 --- a/src/http.cc +++ b/src/http.cc @@ -19,6 +19,8 @@ #include +#include +#include #include #include #include @@ -35,6 +37,7 @@ #include #include #include +#include #include #include "miniocpp/error.h" @@ -53,6 +56,11 @@ namespace minio::http { namespace { +// Abort a transfer that makes no progress for this long. Guards against a +// connection that drops mid-transfer without a clean close (TCP never RSTs), +// which would otherwise keep the request alive indefinitely. +constexpr long kStallTimeoutSecs = 60; + // curl_global_init() is documented as not thread-safe and is expensive // (OpenSSL init etc). Run it exactly once per process via a function-local // static (Meyers singleton; C++11 [stmt.dcl]/4 guarantees thread-safe @@ -403,6 +411,14 @@ Response Request::execute() { curl_easy_setopt(raw_handle, CURLOPT_SHARE, GlobalCurlShare()); curl_easy_setopt(raw_handle, CURLOPT_TCP_KEEPALIVE, 1L); + // Fail a stalled transfer instead of hanging forever. Skipped when the caller + // set an explicit total timeout (RDMA control plane) — that already bounds + // it. + if (timeout_secs <= 0) { + curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_LIMIT, 1L); + curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_TIME, kStallTimeoutSecs); + } + // Request settings. request.setOpt(new curlpp::options::CustomRequest{MethodToString(method)}); std::string urlstring = url.String(); @@ -503,7 +519,7 @@ Response Request::execute() { fd_set fdread{}; fd_set fdwrite{}; fd_set fdexcep{}; - int maxfd = 0; + int maxfd = -1; FD_ZERO(&fdread); FD_ZERO(&fdwrite); @@ -511,14 +527,42 @@ Response Request::execute() { requests.fdset(&fdread, &fdwrite, &fdexcep, &maxfd); - if (select(maxfd + 1, &fdread, &fdwrite, &fdexcep, nullptr) < 0) { - std::cerr << "select() failed; this should not happen" << std::endl; - std::terminate(); + // Bound the wait so the loop keeps pumping libcurl even when no socket ever + // becomes ready — otherwise a dropped/stalled connection blocks select() + // forever and this (synchronous) call hangs the calling thread. The bounded + // poll lets libcurl enforce its own timeouts (e.g. the low-speed limit set + // above) and abort the dead transfer. + if (maxfd < 0) { + // libcurl has no fd to wait on yet; select() with empty sets errors out + // on Windows, so just poll again shortly. + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } else { + timeval timeout{}; + timeout.tv_sec = 1; + timeout.tv_usec = 0; + if (select(maxfd + 1, &fdread, &fdwrite, &fdexcep, &timeout) < 0) { +#ifndef _WIN32 + if (errno == EINTR) continue; // interrupted by a signal; retry +#endif + std::cerr << "select() failed; this should not happen" << std::endl; + std::terminate(); + } } while (!requests.perform(&left)) { } } + // The loop exits once libcurl has no running transfers left. If the transfer + // aborted before delivering a single byte (e.g. the low-speed limit or + // connect timeout fired on a dropped/stalled connection), the write callback + // never ran, so neither status_code nor error was set. Surface a diagnostic + // instead of returning a silently-empty failure. + if (response.error.empty() && response.status_code == 0) { + response.error = + "transfer ended without a response (connection dropped, timed out, or " + "was aborted before any data was received)"; + } + if (progressfunc != nullptr) { ProgressFunctionArgs args; args.userdata = progress_userdata;