From bde2f09d5e4c4a3b2826aefdda0a10c07bbb551a Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 2 Aug 2016 10:57:30 +0200 Subject: multi: make Curl_expire() work with 0 ms timeouts Previously, passing a timeout of zero to Curl_expire() was a magic code for clearing all timeouts for the handle. That is now instead made with the new Curl_expire_clear() function and thus a 0 timeout is fine to set and will trigger a timeout ASAP. This will help removing short delays, in particular notable when doing HTTP/2. --- lib/easy.c | 2 +- lib/http2.c | 9 ++-- lib/multi.c | 158 ++++++++++++++++++++++++++++++--------------------------- lib/multiif.h | 1 + lib/pipeline.c | 6 +-- lib/ssh.c | 2 +- lib/url.c | 2 +- 7 files changed, 95 insertions(+), 85 deletions(-) (limited to 'lib') diff --git a/lib/easy.c b/lib/easy.c index dc7139f23..583de154b 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1044,7 +1044,7 @@ CURLcode curl_easy_pause(struct Curl_easy *data, int action) if(!result && ((newstate&(KEEP_RECV_PAUSE|KEEP_SEND_PAUSE)) != (KEEP_RECV_PAUSE|KEEP_SEND_PAUSE)) ) - Curl_expire(data, 1); /* get this handle going again */ + Curl_expire(data, 0); /* get this handle going again */ return result; } diff --git a/lib/http2.c b/lib/http2.c index efc082dd5..8e10c6601 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -547,7 +547,7 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, /* if we receive data for another handle, wake that up */ if(conn_s->data != data_s) - Curl_expire(data_s, 1); + Curl_expire(data_s, 0); } break; case NGHTTP2_PUSH_PROMISE: @@ -621,8 +621,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, /* if we receive data for another handle, wake that up */ if(conn->data != data_s) - Curl_expire(data_s, 1); /* TODO: fix so that this can be set to 0 for - immediately? */ + Curl_expire(data_s, 0); DEBUGF(infof(data_s, "%zu data received for stream %u " "(%zu left in buffer %p, total %zu)\n", @@ -883,7 +882,7 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, Curl_add_buffer(stream->header_recvbuf, " \r\n", 3); /* if we receive data for another handle, wake that up */ if(conn->data != data_s) - Curl_expire(data_s, 1); + Curl_expire(data_s, 0); DEBUGF(infof(data_s, "h2 status: HTTP/2 %03d (easy %p)\n", stream->status_code, data_s)); @@ -899,7 +898,7 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, Curl_add_buffer(stream->header_recvbuf, "\r\n", 2); /* if we receive data for another handle, wake that up */ if(conn->data != data_s) - Curl_expire(data_s, 1); + Curl_expire(data_s, 0); DEBUGF(infof(data_s, "h2 header: %.*s: %.*s\n", namelen, name, valuelen, value)); diff --git a/lib/multi.c b/lib/multi.c index c471c48d9..7a25103de 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -442,7 +442,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi, sockets that time-out or have actions will be dealt with. Since this handle has no action yet, we make sure it times out to get things to happen. */ - Curl_expire(data, 1); + Curl_expire(data, 0); /* increase the node-counter */ multi->num_easy++; @@ -695,7 +695,7 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, /* The timer must be shut down before data->multi is set to NULL, else the timenode will remain in the splay tree after curl_easy_cleanup is called. */ - Curl_expire(data, 0); + Curl_expire_clear(data); if(data->dns.hostcachetype == HCACHE_MULTI) { /* stop using the multi handle's DNS cache */ @@ -1901,7 +1901,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* expire the new receiving pipeline head */ if(data->easy_conn->recv_pipe->head) - Curl_expire_latest(data->easy_conn->recv_pipe->head->ptr, 1); + Curl_expire_latest(data->easy_conn->recv_pipe->head->ptr, 0); /* Check if we can move pending requests to send pipe */ Curl_multi_process_pending_handles(multi); @@ -2011,7 +2011,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, that could be freed anytime */ data->easy_conn = NULL; - Curl_expire(data, 0); /* stop all timers */ + Curl_expire_clear(data); /* stop all timers */ break; case CURLM_STATE_MSGSENT: @@ -2879,92 +2879,59 @@ multi_addtimeout(struct curl_llist *timeoutlist, * given a number of milliseconds from now to use to set the 'act before * this'-time for the transfer, to be extracted by curl_multi_timeout() * - * Note that the timeout will be added to a queue of timeouts if it defines a - * moment in time that is later than the current head of queue. - * - * Pass zero to clear all timeout values for this handle. -*/ + * The timeout will be added to a queue of timeouts if it defines a moment in + * time that is later than the current head of queue. + */ void Curl_expire(struct Curl_easy *data, long milli) { struct Curl_multi *multi = data->multi; struct timeval *nowp = &data->state.expiretime; int rc; + struct timeval set; /* this is only interesting while there is still an associated multi struct remaining! */ if(!multi) return; - if(!milli) { - /* No timeout, clear the time data. */ - if(nowp->tv_sec || nowp->tv_usec) { - /* Since this is an cleared time, we must remove the previous entry from - the splay tree */ - struct curl_llist *list = data->state.timeoutlist; - - rc = Curl_splayremovebyaddr(multi->timetree, - &data->state.timenode, - &multi->timetree); - if(rc) - infof(data, "Internal error clearing splay node = %d\n", rc); - - /* flush the timeout list too */ - while(list->size > 0) - Curl_llist_remove(list, list->tail, NULL); + set = Curl_tvnow(); + set.tv_sec += milli/1000; + set.tv_usec += (milli%1000)*1000; -#ifdef DEBUGBUILD - infof(data, "Expire cleared\n"); -#endif - nowp->tv_sec = 0; - nowp->tv_usec = 0; - } + if(set.tv_usec >= 1000000) { + set.tv_sec++; + set.tv_usec -= 1000000; } - else { - 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(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. */ - long diff = curlx_tvdiff(set, *nowp); - if(diff > 0) { - /* the new expire time was later so just add it to the queue - and get out */ - multi_addtimeout(data->state.timeoutlist, &set); - 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->state.timeoutlist, nowp); - - /* 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, - &data->state.timenode, - &multi->timetree); - if(rc) - infof(data, "Internal error removing splay node = %d\n", rc); + 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. */ + long diff = curlx_tvdiff(set, *nowp); + if(diff > 0) { + /* the new expire time was later so just add it to the queue + and get out */ + multi_addtimeout(data->state.timeoutlist, &set); + return; } - *nowp = set; - data->state.timenode.payload = data; - multi->timetree = Curl_splayinsert(*nowp, - multi->timetree, - &data->state.timenode); + /* the new time is newer than the presently set one, so add the current + to the queue and update the head */ + multi_addtimeout(data->state.timeoutlist, nowp); + + /* 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, + &data->state.timenode, + &multi->timetree); + if(rc) + infof(data, "Internal error removing splay node = %d\n", rc); } -#if 0 - Curl_splayprint(multi->timetree, 0, TRUE); -#endif + + *nowp = set; + data->state.timenode.payload = data; + multi->timetree = Curl_splayinsert(*nowp, multi->timetree, + &data->state.timenode); } /* @@ -3007,6 +2974,49 @@ void Curl_expire_latest(struct Curl_easy *data, long milli) Curl_expire(data, milli); } + +/* + * Curl_expire_clear() + * + * Clear ALL timeout values for this handle. + */ +void Curl_expire_clear(struct Curl_easy *data) +{ + struct Curl_multi *multi = data->multi; + struct timeval *nowp = &data->state.expiretime; + int rc; + + /* this is only interesting while there is still an associated multi struct + remaining! */ + if(!multi) + return; + + if(nowp->tv_sec || nowp->tv_usec) { + /* Since this is an cleared time, we must remove the previous entry from + the splay tree */ + struct curl_llist *list = data->state.timeoutlist; + + rc = Curl_splayremovebyaddr(multi->timetree, + &data->state.timenode, + &multi->timetree); + if(rc) + infof(data, "Internal error clearing splay node = %d\n", rc); + + /* flush the timeout list too */ + while(list->size > 0) + Curl_llist_remove(list, list->tail, NULL); + +#ifdef DEBUGBUILD + infof(data, "Expire cleared\n"); +#endif + nowp->tv_sec = 0; + nowp->tv_usec = 0; + } +} + + + + CURLMcode curl_multi_assign(struct Curl_multi *multi, curl_socket_t s, void *hashp) { @@ -3067,7 +3077,7 @@ void Curl_multi_process_pending_handles(struct Curl_multi *multi) Curl_llist_remove(multi->pending, e, NULL); /* Make sure that the handle will be processed soonish. */ - Curl_expire_latest(data, 1); + Curl_expire_latest(data, 0); } e = next; /* operate on next handle */ diff --git a/lib/multiif.h b/lib/multiif.h index fd2df556a..eaff496ea 100644 --- a/lib/multiif.h +++ b/lib/multiif.h @@ -26,6 +26,7 @@ * Prototypes for library-wide functions provided by multi.c */ void Curl_expire(struct Curl_easy *data, long milli); +void Curl_expire_clear(struct Curl_easy *data); void Curl_expire_latest(struct Curl_easy *data, long milli); bool Curl_pipeline_wanted(const struct Curl_multi* multi, int bits); void Curl_multi_handlePipeBreak(struct Curl_easy *data); diff --git a/lib/pipeline.c b/lib/pipeline.c index 0ff82f086..bd902d9a3 100644 --- a/lib/pipeline.c +++ b/lib/pipeline.c @@ -6,7 +6,7 @@ * \___|\___/|_| \_\_____| * * Copyright (C) 2013, Linus Nielsen Feltzing, - * Copyright (C) 2013-2015, Daniel Stenberg, , et al. + * Copyright (C) 2013-2016, 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 @@ -114,7 +114,7 @@ CURLcode Curl_add_handle_to_pipeline(struct Curl_easy *handle, if(pipeline == conn->send_pipe && sendhead != conn->send_pipe->head) { /* this is a new one as head, expire it */ Curl_pipeline_leave_write(conn); /* not in use yet */ - Curl_expire(conn->send_pipe->head->ptr, 1); + Curl_expire(conn->send_pipe->head->ptr, 0); } #if 0 /* enable for pipeline debugging */ @@ -149,7 +149,7 @@ void Curl_move_handle_from_send_to_recv_pipe(struct Curl_easy *handle, infof(conn->data, "%p is at send pipe head B!\n", (void *)conn->send_pipe->head->ptr); #endif - Curl_expire(conn->send_pipe->head->ptr, 1); + Curl_expire(conn->send_pipe->head->ptr, 0); } /* The receiver's list is not really interesting here since either this diff --git a/lib/ssh.c b/lib/ssh.c index 7bc313622..bf7cfe01b 100644 --- a/lib/ssh.c +++ b/lib/ssh.c @@ -1895,7 +1895,7 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block) /* since we don't really wait for anything at this point, we want the state machine to move on as soon as possible so we set a very short timeout here */ - Curl_expire(data, 1); + Curl_expire(data, 0); state(conn, SSH_STOP); } diff --git a/lib/url.c b/lib/url.c index e547e5c16..da48da6b9 100644 --- a/lib/url.c +++ b/lib/url.c @@ -405,7 +405,7 @@ CURLcode Curl_close(struct Curl_easy *data) if(!data) return CURLE_OK; - Curl_expire(data, 0); /* shut off timers */ + Curl_expire_clear(data); /* shut off timers */ m = data->multi; -- cgit v1.2.3