aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrad Spencer <bspencer@blackberry.com>2017-07-29 16:44:39 +0200
committerDaniel Stenberg <daniel@haxx.se>2017-08-01 13:39:38 +0200
commit164a09368d8a95f4a5c5e4b63420e42d261991f2 (patch)
tree9484b544f11aa7ce0e051b6cd722b63acb2e05bb
parent53d137d94ac355bd3fa1717141e688d3534f21f7 (diff)
multi: fix request timer management
There are some bugs in how timers are managed for a single easy handle that causes the wrong "next timeout" value to be reported to the application when a new minimum needs to be recomputed and that new minimum should be an existing timer that isn't currently set for the easy handle. When the application drives a set of easy handles via the `curl_multi_socket_action()` API (for example), it gets told to wait the wrong amount of time before the next call, which causes requests to linger for a long time (or, it is my guess, possibly forever). Bug: https://curl.haxx.se/mail/lib-2017-07/0033.html
-rw-r--r--lib/multi.c27
1 files changed, 13 insertions, 14 deletions
diff --git a/lib/multi.c b/lib/multi.c
index e29d99272..d5bc532ea 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2532,10 +2532,8 @@ static CURLMcode add_next_timeout(struct curltime now,
/* copy the first entry to 'tv' */
memcpy(tv, &node->time, sizeof(*tv));
- /* remove first entry from list */
- Curl_llist_remove(list, e, NULL);
-
- /* insert this node again into the splay */
+ /* Insert this node again into the splay. Keep the timer in the list in
+ case we need to recompute future timers. */
multi->timetree = Curl_splayinsert(*tv, multi->timetree,
&d->state.timenode);
}
@@ -2952,26 +2950,25 @@ void Curl_expire(struct Curl_easy *data, time_t milli, expire_id id)
set.tv_usec -= 1000000;
}
+ /* Remove any timer with the same id just in case. */
+ multi_deltimeout(data, id);
+
+ /* Add it to the timer list. It must stay in the list until it has expired
+ in case we need to recompute the minimum timer later. */
+ multi_addtimeout(data, &set, id);
+
if(nowp->tv_sec || nowp->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. */
time_t diff = curlx_tvdiff(set, *nowp);
- /* remove the previous timer first, if there */
- multi_deltimeout(data, id);
-
if(diff > 0) {
- /* the new expire time was later so just add it to the queue
- and get out */
- multi_addtimeout(data, &set, id);
+ /* The current splay tree entry is sooner than this new expiry time.
+ We don't need to update our splay tree entry. */
return;
}
- /* the new time is newer than the presently set one, so add the current
- to the queue and update the head */
- multi_addtimeout(data, nowp, id);
-
/* Since this is an updated time, we must remove the previous entry from
the splay tree first and then re-add the new value */
rc = Curl_splayremovebyaddr(multi->timetree,
@@ -2981,6 +2978,8 @@ void Curl_expire(struct Curl_easy *data, time_t milli, expire_id id)
infof(data, "Internal error removing splay node = %d\n", rc);
}
+ /* Indicate that we are in the splay tree and insert the new timer expiry
+ value since it is our local minimum. */
*nowp = set;
data->state.timenode.payload = data;
multi->timetree = Curl_splayinsert(*nowp, multi->timetree,