From f07563aba99334e36b64118b5cb7cb5dab7173d9 Mon Sep 17 00:00:00 2001 From: Peter Keuter Date: Tue, 23 Jun 2026 10:37:09 +0200 Subject: [PATCH 1/5] fix: stop uploads hanging forever when the connection drops mid-transfer The libcurl multi-pump loop called select() with a nullptr timeout (block forever). A connection that drops mid-upload without a clean RST/FIN never makes the socket ready, so select() blocked indefinitely and the synchronous Execute() wedged the calling thread permanently -- no further uploads could run. Normal uploads also had no timeout configured (only the RDMA control plane set one), so libcurl had nothing to enforce even when woken. - Bound the select() wait to 1s so the loop keeps pumping libcurl. - Handle maxfd < 0 with a short poll-sleep instead of an invalid select() call (which also errors out / terminates on Windows). - Set a default low-speed timeout (1 byte/s for 60s) so a stalled transfer aborts with CURLE_OPERATION_TIMEDOUT. Gated on timeout_secs <= 0 to leave the RDMA control plane's explicit timeouts unchanged. --- src/http.cc | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/http.cc b/src/http.cc index 572803aa..d0b8cc03 100644 --- a/src/http.cc +++ b/src/http.cc @@ -25,12 +25,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -53,6 +55,12 @@ 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. +// ponytail: fixed default; promote to a Request field if callers need it tunable. +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,13 @@ 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 +518,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,9 +526,23 @@ 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) { + std::cerr << "select() failed; this should not happen" << std::endl; + std::terminate(); + } } while (!requests.perform(&left)) { } From 9644c685ca39750e75ed3066c8b620b79022c278 Mon Sep 17 00:00:00 2001 From: Peter Keuter Date: Fri, 26 Jun 2026 08:29:10 +0200 Subject: [PATCH 2/5] fix: linting --- src/http.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/http.cc b/src/http.cc index d0b8cc03..36be73fc 100644 --- a/src/http.cc +++ b/src/http.cc @@ -19,24 +19,24 @@ #include +#include #include #include #include #include #include #include -#include #include #include #include #include #include #include -#include #include #include #include #include +#include #include #include "miniocpp/error.h" @@ -58,7 +58,6 @@ 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. -// ponytail: fixed default; promote to a Request field if callers need it tunable. constexpr long kStallTimeoutSecs = 60; // curl_global_init() is documented as not thread-safe and is expensive @@ -412,7 +411,8 @@ Response Request::execute() { 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. + // 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); @@ -532,8 +532,8 @@ Response Request::execute() { // 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. + // 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{}; From ba095552107d4ad168e684263a1f3b1ef1b030dd Mon Sep 17 00:00:00 2001 From: Peter Keuter Date: Fri, 26 Jun 2026 10:43:25 +0200 Subject: [PATCH 3/5] fix: coderabbit warning --- src/http.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/http.cc b/src/http.cc index 36be73fc..a21d9295 100644 --- a/src/http.cc +++ b/src/http.cc @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -540,6 +541,9 @@ Response Request::execute() { 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(); } From c34e26cd3b8f8cad530cc0aaf786a99d4af586e4 Mon Sep 17 00:00:00 2001 From: Peter Keuter Date: Fri, 26 Jun 2026 11:05:01 +0200 Subject: [PATCH 4/5] fix: another coderabbit warning (error swallowing) --- src/http.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/http.cc b/src/http.cc index a21d9295..30f7f998 100644 --- a/src/http.cc +++ b/src/http.cc @@ -552,6 +552,17 @@ Response Request::execute() { } } + // 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; From dd58c10584818428c7b79085c3c1dacb6d63bcca Mon Sep 17 00:00:00 2001 From: Peter Keuter Date: Fri, 26 Jun 2026 11:18:36 +0200 Subject: [PATCH 5/5] chore: updated doc --- include/miniocpp/http.h | 6 ++++++ 1 file changed, 6 insertions(+) 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;