From 7383e925273fd54850f2a7d6365e5e4e7600ddb2 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Mon, 25 Aug 2014 11:34:14 +0200
Subject: [PATCH 1/2] 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
Upstream-commit: cacdc27f52ba7b0bf08aa57886bfbd18bc82ebfb
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
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 | 4 ++--
6 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c
index 081667d..f14e3e1 100644
--- a/lib/asyn-ares.c
+++ b/lib/asyn-ares.c
@@ -235,7 +235,7 @@ int Curl_resolver_getsock(struct connectdata *conn,
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 66fb510..bc94c5d 100644
--- a/lib/asyn-thread.c
+++ b/lib/asyn-thread.c
@@ -502,7 +502,7 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn,
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 413c66e..0fea766 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -1016,7 +1016,7 @@ singleipconnect(struct connectdata *conn,
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)) {
diff --git a/lib/multi.c b/lib/multi.c
index 476f058..6fc5bb1 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1454,7 +1454,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
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;
}
@@ -1470,7 +1470,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
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;
}
@@ -1532,7 +1532,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
/* expire the new receiving pipeline head */
if(easy->easy_conn->recv_pipe->head)
- Curl_expire(easy->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);
@@ -2644,6 +2644,46 @@ void Curl_expire(struct SessionHandle *data, long milli)
#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)
{
diff --git a/lib/multiif.h b/lib/multiif.h
index d1b0e2f..9c00cc8 100644
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -28,6 +28,7 @@
* 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);
diff --git a/lib/speedcheck.c b/lib/speedcheck.c
index ea17a59..a9b421d 100644
--- a/lib/speedcheck.c
+++ b/lib/speedcheck.c
@@ -57,7 +57,7 @@ CURLcode Curl_speedcheck(struct SessionHandle *data,
}
else {
/* wait complete low_speed_time */
- Curl_expire(data, nextcheck);
+ Curl_expire_latest(data, nextcheck);
}
}
else {
@@ -68,7 +68,7 @@ CURLcode Curl_speedcheck(struct SessionHandle *data,
/* 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
From b7abc7d64288949b7e1f955930e7f492ac1000d5 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 23 Sep 2014 11:44:03 +0200
Subject: [PATCH 2/2] threaded-resolver: revert Curl_expire_latest() switch
The switch to using Curl_expire_latest() in commit cacdc27f52b was a
mistake and was against the advice even mentioned in that commit. The
comparison in asyn-thread.c:Curl_resolver_is_resolved() makes
Curl_expire() the suitable function to use.
Bug: http://curl.haxx.se/bug/view.cgi?id=1426
Reported-By: graysky
Upstream-commit: d9762a7cdb35e70f8cb0bf1c2f8019e8391616e1
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/asyn-thread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c
index 7fe6472..c72c064 100644
--- a/lib/asyn-thread.c
+++ b/lib/asyn-thread.c
@@ -502,7 +502,7 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn,
td->poll_interval = 250;
td->interval_end = elapsed + td->poll_interval;
- Curl_expire_latest(conn->data, td->poll_interval);
+ Curl_expire(conn->data, td->poll_interval);
}
return CURLE_OK;
--
2.1.0