From b683623d699af2cf5ba405ae4ac288c3d3c94011 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 25 Aug 2014 11:34:14 +0200 Subject: [PATCH] low-speed-limit: avoid timeout flood Introducing Curl_expire_latest(). To be used when we the code flow only wants to get called at a later time that is "no later than X" so that something can be checked (and another timeout be added). The low-speed logic for example could easily be made to set very many expire timeouts if it would be called faster or sooner than what it had set its own timer and this goes for a few other timers too that aren't explictiy checked for timer expiration in the code. If there's no condition the code that says if(time-passed >= TIME), then Curl_expire_latest() is preferred to Curl_expire(). If there exists such a condition, it is on the other hand important that Curl_expire() is used and not the other. Bug: http://curl.haxx.se/mail/lib-2014-06/0235.html Reported-by: Florian Weimer --- lib/asyn-ares.c | 2 +- lib/asyn-thread.c | 2 +- lib/connect.c | 2 +- lib/multi.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- lib/multiif.h | 1 + lib/speedcheck.c | 6 +++--- 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c index cb99a1f..01a9c9b 100644 --- a/lib/asyn-ares.c +++ b/lib/asyn-ares.c @@ -233,11 +233,11 @@ int Curl_resolver_getsock(struct connectdata *conn, timeout = ares_timeout((ares_channel)conn->data->state.resolver, &maxtime, &timebuf); milli = (timeout->tv_sec * 1000) + (timeout->tv_usec/1000); if(milli == 0) milli += 10; - Curl_expire(conn->data, milli); + Curl_expire_latest(conn->data, milli); return max; } /* diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c index 31a3132..a725778 100644 --- a/lib/asyn-thread.c +++ b/lib/asyn-thread.c @@ -501,11 +501,11 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn, if(td->poll_interval > 250) td->poll_interval = 250; td->interval_end = elapsed + td->poll_interval; - Curl_expire(conn->data, td->poll_interval); + Curl_expire_latest(conn->data, td->poll_interval); } return CURLE_OK; } diff --git a/lib/connect.c b/lib/connect.c index 6a79e64..fb315fc 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -1052,11 +1052,11 @@ singleipconnect(struct connectdata *conn, /* set socket non-blocking */ curlx_nonblock(sockfd, TRUE); conn->connecttime = Curl_tvnow(); if(conn->num_addr > 1) - Curl_expire(data, conn->timeoutms_per_addr); + Curl_expire_latest(data, conn->timeoutms_per_addr); /* Connect TCP sockets, bind UDP */ if(!isconnected && (conn->socktype == SOCK_STREAM)) { rc = connect(sockfd, &addr.sa_addr, addr.addrlen); if(-1 == rc) diff --git a/lib/multi.c b/lib/multi.c index 1e5b3c8..5106e7e 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1465,11 +1465,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* calculate upload rate-limitation timeout. */ buffersize = (int)(data->set.buffer_size ? data->set.buffer_size : BUFSIZE); timeout_ms = Curl_sleep_time(data->set.max_send_speed, data->progress.ulspeed, buffersize); - Curl_expire(data, timeout_ms); + Curl_expire_latest(data, timeout_ms); break; } /* check if over recv speed */ if((data->set.max_recv_speed > 0) && @@ -1481,11 +1481,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* Calculate download rate-limitation timeout. */ buffersize = (int)(data->set.buffer_size ? data->set.buffer_size : BUFSIZE); timeout_ms = Curl_sleep_time(data->set.max_recv_speed, data->progress.dlspeed, buffersize); - Curl_expire(data, timeout_ms); + Curl_expire_latest(data, timeout_ms); break; } /* read/write data if it is ready to do so */ data->result = Curl_readwrite(data->easy_conn, &done); @@ -1543,11 +1543,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* we're no longer receiving */ Curl_removeHandleFromPipeline(data, data->easy_conn->recv_pipe); /* expire the new receiving pipeline head */ if(data->easy_conn->recv_pipe->head) - Curl_expire(data->easy_conn->recv_pipe->head->ptr, 1); + Curl_expire_latest(data->easy_conn->recv_pipe->head->ptr, 1); /* Check if we can move pending requests to send pipe */ Curl_multi_process_pending_handles(multi); /* When we follow redirects or is set to retry the connection, we must @@ -2656,10 +2656,50 @@ void Curl_expire(struct SessionHandle *data, long milli) #if 0 Curl_splayprint(multi->timetree, 0, TRUE); #endif } +/* + * Curl_expire_latest() + * + * This is like Curl_expire() but will only add a timeout node to the list of + * timers if there is no timeout that will expire before the given time. + * + * Use this function if the code logic risks calling this function many times + * or if there's no particular conditional wait in the code for this specific + * time-out period to expire. + * + */ +void Curl_expire_latest(struct SessionHandle *data, long milli) +{ + struct timeval *exp = &data->state.expiretime; + + struct timeval set; + + set = Curl_tvnow(); + set.tv_sec += milli/1000; + set.tv_usec += (milli%1000)*1000; + + if(set.tv_usec >= 1000000) { + set.tv_sec++; + set.tv_usec -= 1000000; + } + + if(exp->tv_sec || exp->tv_usec) { + /* This means that the struct is added as a node in the splay tree. + Compare if the new time is earlier, and only remove-old/add-new if it + is. */ + long diff = curlx_tvdiff(set, *exp); + if(diff > 0) + /* the new expire time was later than the top time, so just skip this */ + return; + } + + /* Just add the timeout like normal */ + Curl_expire(data, milli); +} + CURLMcode curl_multi_assign(CURLM *multi_handle, curl_socket_t s, void *hashp) { struct Curl_sh_entry *there = NULL; struct Curl_multi *multi = (struct Curl_multi *)multi_handle; diff --git a/lib/multiif.h b/lib/multiif.h index 1cbd310..c77b3ca 100644 --- a/lib/multiif.h +++ b/lib/multiif.h @@ -24,10 +24,11 @@ /* * Prototypes for library-wide functions provided by multi.c */ void Curl_expire(struct SessionHandle *data, long milli); +void Curl_expire_latest(struct SessionHandle *data, long milli); bool Curl_multi_pipeline_enabled(const struct Curl_multi* multi); void Curl_multi_handlePipeBreak(struct SessionHandle *data); /* Internal version of curl_multi_init() accepts size parameters for the diff --git a/lib/speedcheck.c b/lib/speedcheck.c index ea17a59..ac7447c 100644 --- a/lib/speedcheck.c +++ b/lib/speedcheck.c @@ -3,11 +3,11 @@ * Project ___| | | | _ \| | * / __| | | | |_) | | * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2011, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2014, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms * are also available at http://curl.haxx.se/docs/copyright.html. * @@ -55,20 +55,20 @@ CURLcode Curl_speedcheck(struct SessionHandle *data, data->set.low_speed_time); return CURLE_OPERATION_TIMEDOUT; } else { /* wait complete low_speed_time */ - Curl_expire(data, nextcheck); + Curl_expire_latest(data, nextcheck); } } else { /* we keep up the required speed all right */ data->state.keeps_speed = now; if(data->set.low_speed_limit) /* if there is a low speed limit enabled, we set the expire timer to make this connection's speed get checked again no later than when this time is up */ - Curl_expire(data, data->set.low_speed_time*1000); + Curl_expire_latest(data, data->set.low_speed_time*1000); } return CURLE_OK; } -- 2.1.0