url: fix connection lifetime checks

The checks for a connection being "too long idle" or "too old" where
rounding down the elapsed time to seconds before comparing to the
configured max values. This caused connections to be reused for up to
999ms longer than intended.

Change the compares to scale the configured seconds up to ms, so
connection will properly be "too old" 1 ms after the coonfigured values.

Fixes sporadic failures of test1542 on platforms where "sleep(2)"
returnes before 2 full seconds on the internal clock where passed.

Reported-by: Christian Weisgerber
URL: https://curl.se/mail/lib-2025-06/0004.html
Closes #17571
This commit is contained in:
Stefan Eissing 2025-06-10 10:11:40 +02:00 committed by Daniel Stenberg
parent f9d8ed63ed
commit e39b8c4819
No known key found for this signature in database
GPG key ID: 5CC908FDB71E12C2
7 changed files with 91 additions and 88 deletions

View file

@ -117,8 +117,8 @@ struct cf_hc_ctx {
CURLcode result; /* overall result */
struct cf_hc_baller ballers[2];
size_t baller_count;
unsigned int soft_eyeballs_timeout_ms;
unsigned int hard_eyeballs_timeout_ms;
timediff_t soft_eyeballs_timeout_ms;
timediff_t hard_eyeballs_timeout_ms;
};
static void cf_hc_baller_assign(struct cf_hc_baller *b,
@ -268,15 +268,16 @@ static bool time_to_start_next(struct Curl_cfilter *cf,
}
elapsed_ms = curlx_timediff(now, ctx->started);
if(elapsed_ms >= ctx->hard_eyeballs_timeout_ms) {
CURL_TRC_CF(data, cf, "hard timeout of %dms reached, starting %s",
CURL_TRC_CF(data, cf, "hard timeout of %" FMT_TIMEDIFF_T "ms reached, "
"starting %s",
ctx->hard_eyeballs_timeout_ms, ctx->ballers[idx].name);
return TRUE;
}
if((idx > 0) && (elapsed_ms >= ctx->soft_eyeballs_timeout_ms)) {
if(cf_hc_baller_reply_ms(&ctx->ballers[idx - 1], data) < 0) {
CURL_TRC_CF(data, cf, "soft timeout of %dms reached, %s has not "
"seen any data, starting %s",
CURL_TRC_CF(data, cf, "soft timeout of %" FMT_TIMEDIFF_T "ms reached, "
"%s has not seen any data, starting %s",
ctx->soft_eyeballs_timeout_ms,
ctx->ballers[idx - 1].name, ctx->ballers[idx].name);
return TRUE;
@ -314,8 +315,8 @@ static CURLcode cf_hc_connect(struct Curl_cfilter *cf,
cf_hc_baller_init(&ctx->ballers[0], cf, data, cf->conn->transport);
if(ctx->baller_count > 1) {
Curl_expire(data, ctx->soft_eyeballs_timeout_ms, EXPIRE_ALPN_EYEBALLS);
CURL_TRC_CF(data, cf, "set next attempt to start in %ums",
ctx->soft_eyeballs_timeout_ms);
CURL_TRC_CF(data, cf, "set next attempt to start in %" FMT_TIMEDIFF_T
"ms", ctx->soft_eyeballs_timeout_ms);
}
ctx->state = CF_HC_CONNECT;
FALLTHROUGH();

View file

@ -172,11 +172,11 @@ void Curl_shutdown_start(struct Curl_easy *data, int sockindex,
}
data->conn->shutdown.start[sockindex] = *nowp;
data->conn->shutdown.timeout_ms = (timeout_ms > 0) ?
(unsigned int)timeout_ms :
(timediff_t)timeout_ms :
((data->set.shutdowntimeout > 0) ?
data->set.shutdowntimeout : DEFAULT_SHUTDOWN_TIMEOUT_MS);
/* Set a timer, unless we operate on the admin handle */
if(data->mid && data->conn->shutdown.timeout_ms)
if(data->mid && (data->conn->shutdown.timeout_ms > 0))
Curl_expire_ex(data, nowp, data->conn->shutdown.timeout_ms,
EXPIRE_SHUTDOWN);
}
@ -187,7 +187,8 @@ timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex,
struct curltime now;
timediff_t left_ms;
if(!conn->shutdown.start[sockindex].tv_sec || !conn->shutdown.timeout_ms)
if(!conn->shutdown.start[sockindex].tv_sec ||
(conn->shutdown.timeout_ms <= 0))
return 0; /* not started or no limits */
if(!nowp) {

View file

@ -1245,7 +1245,7 @@ out:
}
data->conn->bits.do_more = FALSE;
Curl_pgrsTime(data, TIMER_STARTACCEPT);
Curl_expire(data, data->set.accepttimeout ?
Curl_expire(data, (data->set.accepttimeout > 0) ?
data->set.accepttimeout: DEFAULT_ACCEPT_TIMEOUT,
EXPIRE_FTP_ACCEPT);
}

View file

@ -52,7 +52,7 @@ timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
struct pingpong *pp, bool disconnecting)
{
timediff_t timeout_ms; /* in milliseconds */
timediff_t response_time = (data->set.server_response_timeout) ?
timediff_t response_time = (data->set.server_response_timeout > 0) ?
data->set.server_response_timeout : pp->response_time;
struct curltime now = curlx_now();
@ -65,7 +65,7 @@ timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
full response to arrive before we bail out */
timeout_ms = response_time - curlx_timediff(now, pp->response);
if(data->set.timeout && !disconnecting) {
if((data->set.timeout > 0) && !disconnecting) {
/* if timeout is requested, find out how much overall remains */
timediff_t timeout2_ms = Curl_timeleft(data, &now, FALSE);
/* pick the lowest number */

View file

@ -60,6 +60,35 @@
#include "curl_memory.h"
#include "memdebug.h"
static CURLcode setopt_set_timeout_sec(timediff_t *ptimeout_ms, long secs)
{
if(secs < 0)
return CURLE_BAD_FUNCTION_ARGUMENT;
#if LONG_MAX > (TIMEDIFF_T_MAX/1000)
if(secs > (TIMEDIFF_T_MAX/1000)) {
*ptimeout_ms = TIMEDIFF_T_MAX;
return CURLE_OK;
}
#endif
*ptimeout_ms = (timediff_t)secs * 1000;
return CURLE_OK;
}
static CURLcode setopt_set_timeout_ms(timediff_t *ptimeout_ms, long ms)
{
if(ms < 0)
return CURLE_BAD_FUNCTION_ARGUMENT;
#if LONG_MAX > TIMEDIFF_T_MAX
if(ms > TIMEDIFF_T_MAX) {
*ptimeout_ms = TIMEDIFF_T_MAX;
return CURLE_OK;
}
#endif
*ptimeout_ms = (timediff_t)ms;
return CURLE_OK;
}
CURLcode Curl_setstropt(char **charp, const char *s)
{
/* Release the previous storage at `charp' and replace by a dynamic storage
@ -526,21 +555,15 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
* Option that specifies how quickly a server response must be obtained
* before it is considered failure. For pingpong protocols.
*/
if((arg >= 0) && (arg <= (INT_MAX/1000)))
data->set.server_response_timeout = (unsigned int)arg * 1000;
else
return CURLE_BAD_FUNCTION_ARGUMENT;
break;
return setopt_set_timeout_sec(&data->set.server_response_timeout, arg);
case CURLOPT_SERVER_RESPONSE_TIMEOUT_MS:
/*
* Option that specifies how quickly a server response must be obtained
* before it is considered failure. For pingpong protocols.
*/
if((arg >= 0) && (arg <= INT_MAX))
data->set.server_response_timeout = (unsigned int)arg;
else
return CURLE_BAD_FUNCTION_ARGUMENT;
break;
return setopt_set_timeout_ms(&data->set.server_response_timeout, arg);
#ifndef CURL_DISABLE_TFTP
case CURLOPT_TFTP_NO_OPTIONS:
/*
@ -883,10 +906,8 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
/*
* The maximum time for curl to wait for FTP server connect
*/
if(uarg > UINT_MAX)
uarg = UINT_MAX;
data->set.accepttimeout = (unsigned int)uarg;
break;
return setopt_set_timeout_ms(&data->set.accepttimeout, arg);
case CURLOPT_WILDCARDMATCH:
data->set.wildcard_enabled = enabled;
break;
@ -943,33 +964,19 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
* The maximum time you allow curl to use for a single transfer
* operation.
*/
if((arg >= 0) && (arg <= (INT_MAX/1000)))
data->set.timeout = (unsigned int)arg * 1000;
else
return CURLE_BAD_FUNCTION_ARGUMENT;
break;
return setopt_set_timeout_sec(&data->set.timeout, arg);
case CURLOPT_TIMEOUT_MS:
if(uarg > UINT_MAX)
uarg = UINT_MAX;
data->set.timeout = (unsigned int)uarg;
break;
return setopt_set_timeout_ms(&data->set.timeout, arg);
case CURLOPT_CONNECTTIMEOUT:
/*
* The maximum time you allow curl to use to connect.
*/
if((arg >= 0) && (arg <= (INT_MAX/1000)))
data->set.connecttimeout = (unsigned int)arg * 1000;
else
return CURLE_BAD_FUNCTION_ARGUMENT;
break;
return setopt_set_timeout_sec(&data->set.connecttimeout, arg);
case CURLOPT_CONNECTTIMEOUT_MS:
if(uarg > UINT_MAX)
uarg = UINT_MAX;
data->set.connecttimeout = (unsigned int)uarg;
break;
return setopt_set_timeout_ms(&data->set.connecttimeout, arg);
case CURLOPT_RESUME_FROM:
/*
@ -1347,10 +1354,8 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
data->set.suppress_connect_headers = enabled;
break;
case CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS:
if(uarg > UINT_MAX)
uarg = UINT_MAX;
data->set.happy_eyeballs_timeout = (unsigned int)uarg;
break;
return setopt_set_timeout_ms(&data->set.happy_eyeballs_timeout, arg);
#ifndef CURL_DISABLE_SHUFFLE_DNS
case CURLOPT_DNS_SHUFFLE_ADDRESSES:
data->set.dns_shuffle_addresses = enabled;
@ -1366,15 +1371,11 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
data->set.upkeep_interval_ms = arg;
break;
case CURLOPT_MAXAGE_CONN:
if(arg < 0)
return CURLE_BAD_FUNCTION_ARGUMENT;
data->set.maxage_conn = arg;
break;
return setopt_set_timeout_sec(&data->set.conn_max_idle_ms, arg);
case CURLOPT_MAXLIFETIME_CONN:
if(arg < 0)
return CURLE_BAD_FUNCTION_ARGUMENT;
data->set.maxlifetime_conn = arg;
break;
return setopt_set_timeout_sec(&data->set.conn_max_age_ms, arg);
#ifndef CURL_DISABLE_HSTS
case CURLOPT_HSTS_CTRL:
if(arg & CURLHSTS_ENABLE) {

View file

@ -473,8 +473,8 @@ CURLcode Curl_init_userdefined(struct Curl_easy *data)
set->happy_eyeballs_timeout = CURL_HET_DEFAULT;
set->upkeep_interval_ms = CURL_UPKEEP_INTERVAL_DEFAULT;
set->maxconnects = DEFAULT_CONNCACHE_SIZE; /* for easy handles */
set->maxage_conn = 118;
set->maxlifetime_conn = 0;
set->conn_max_idle_ms = 118 * 1000;
set->conn_max_age_ms = 0;
set->http09_allowed = FALSE;
set->httpwant = CURL_HTTP_VERSION_NONE
;
@ -671,36 +671,36 @@ socks_proxy_info_matches(const struct proxy_info *data,
#define socks_proxy_info_matches(x,y) FALSE
#endif
/* A connection has to have been idle for a shorter time than 'maxage_conn'
/* A connection has to have been idle for less than 'conn_max_idle_ms'
(the success rate is just too low after this), or created less than
'maxlifetime_conn' ago, to be subject for reuse. */
'conn_max_age_ms' ago, to be subject for reuse. */
static bool conn_maxage(struct Curl_easy *data,
struct connectdata *conn,
struct curltime now)
{
timediff_t idletime, lifetime;
timediff_t age_ms;
idletime = curlx_timediff(now, conn->lastused);
idletime /= 1000; /* integer seconds is fine */
if(idletime > data->set.maxage_conn) {
infof(data, "Too old connection (%" FMT_TIMEDIFF_T
" seconds idle), disconnect it", idletime);
return TRUE;
if(data->set.conn_max_idle_ms) {
age_ms = curlx_timediff(now, conn->lastused);
if(age_ms > data->set.conn_max_idle_ms) {
infof(data, "Too old connection (%" FMT_TIMEDIFF_T
" ms idle, max idle is %" FMT_TIMEDIFF_T " ms), disconnect it",
age_ms, data->set.conn_max_idle_ms);
return TRUE;
}
}
lifetime = curlx_timediff(now, conn->created);
lifetime /= 1000; /* integer seconds is fine */
if(data->set.maxlifetime_conn && lifetime > data->set.maxlifetime_conn) {
infof(data,
"Too old connection (%" FMT_TIMEDIFF_T
" seconds since creation), disconnect it", lifetime);
return TRUE;
if(data->set.conn_max_age_ms) {
age_ms = curlx_timediff(now, conn->created);
if(age_ms > data->set.conn_max_age_ms) {
infof(data,
"Too old connection (created %" FMT_TIMEDIFF_T
" ms ago, max lifetime is %" FMT_TIMEDIFF_T " ms), disconnect it",
age_ms, data->set.conn_max_age_ms);
return TRUE;
}
}
return FALSE;
}

View file

@ -701,7 +701,7 @@ struct connectdata {
struct Curl_cfilter *cfilter[2]; /* connection filters */
struct {
struct curltime start[2]; /* when filter shutdown started */
unsigned int timeout_ms; /* 0 means no timeout */
timediff_t timeout_ms; /* 0 means no timeout */
} shutdown;
struct ssl_primary_config ssl_config;
@ -1412,9 +1412,9 @@ struct UserDefined {
#endif
void *progress_client; /* pointer to pass to the progress callback */
void *ioctl_client; /* pointer to pass to the ioctl callback */
long maxage_conn; /* in seconds, max idle time to allow a connection that
timediff_t conn_max_idle_ms; /* max idle time to allow a connection that
is to be reused */
long maxlifetime_conn; /* in seconds, max time since creation to allow a
timediff_t conn_max_age_ms; /* max time since creation to allow a
connection that is to be reused */
#ifndef CURL_DISABLE_TFTP
long tftp_blksize; /* in bytes, 0 means use default */
@ -1460,7 +1460,7 @@ struct UserDefined {
#endif
curl_off_t max_filesize; /* Maximum file size to download */
#ifndef CURL_DISABLE_FTP
unsigned int accepttimeout; /* in milliseconds, 0 means no timeout */
timediff_t accepttimeout; /* in milliseconds, 0 means no timeout */
unsigned char ftp_filemethod; /* how to get to a file: curl_ftpfile */
unsigned char ftpsslauth; /* what AUTH XXX to try: curl_ftpauth */
unsigned char ftp_ccc; /* FTP CCC options: curl_ftpccc */
@ -1504,11 +1504,11 @@ struct UserDefined {
void *wildcardptr;
#endif
unsigned int timeout; /* ms, 0 means no timeout */
unsigned int connecttimeout; /* ms, 0 means default timeout */
unsigned int happy_eyeballs_timeout; /* ms, 0 is a valid value */
unsigned int server_response_timeout; /* ms, 0 means no timeout */
unsigned int shutdowntimeout; /* ms, 0 means default timeout */
timediff_t timeout; /* ms, 0 means no timeout */
timediff_t connecttimeout; /* ms, 0 means default timeout */
timediff_t happy_eyeballs_timeout; /* ms, 0 is a valid value */
timediff_t server_response_timeout; /* ms, 0 means no timeout */
timediff_t shutdowntimeout; /* ms, 0 means default timeout */
int tcp_keepidle; /* seconds in idle before sending keepalive probe */
int tcp_keepintvl; /* seconds between TCP keepalive probes */
int tcp_keepcnt; /* maximum number of keepalive probes */