mirror of https://github.com/moparisthebest/curl
- Emil Romanus found a problem and helped me repeat it. It occured when using
the curl_multi_socket() API with HTTP pipelining enabled and could lead to the pipeline basically stalling for a very long period of time until it took off again.
This commit is contained in:
parent
b49dcfb52b
commit
ec4f6e93c2
5
CHANGES
5
CHANGES
|
@ -8,6 +8,11 @@
|
||||||
|
|
||||||
|
|
||||||
Daniel Stenberg (28 May 2008)
|
Daniel Stenberg (28 May 2008)
|
||||||
|
- Emil Romanus found a problem and helped me repeat it. It occured when using
|
||||||
|
the curl_multi_socket() API with HTTP pipelining enabled and could lead to
|
||||||
|
the pipeline basically stalling for a very long period of time until it took
|
||||||
|
off again.
|
||||||
|
|
||||||
- Jeff Weber reported memory leaks with aborted SCP and SFTP transfers and
|
- Jeff Weber reported memory leaks with aborted SCP and SFTP transfers and
|
||||||
provided excellent repeat recipes. I fixed the cases I managed to reproduce
|
provided excellent repeat recipes. I fixed the cases I managed to reproduce
|
||||||
but Jeff still got some (SCP) problems even after these fixes:
|
but Jeff still got some (SCP) problems even after these fixes:
|
||||||
|
|
|
@ -39,6 +39,7 @@ This release includes the following bugfixes:
|
||||||
o builds fine for Haiku OS
|
o builds fine for Haiku OS
|
||||||
o follow redirect with only a new query string
|
o follow redirect with only a new query string
|
||||||
o SCP and SFTP memory leaks on aborted transfers
|
o SCP and SFTP memory leaks on aborted transfers
|
||||||
|
o curl_multi_socket() and HTTP pipelining transfer stalls
|
||||||
|
|
||||||
This release includes the following known bugs:
|
This release includes the following known bugs:
|
||||||
|
|
||||||
|
@ -61,6 +62,6 @@ advice from friends like these:
|
||||||
Rafa Muyo, Andre Guibert de Bruet, Brock Noland, Sandor Feldi, Stefan Krause,
|
Rafa Muyo, Andre Guibert de Bruet, Brock Noland, Sandor Feldi, Stefan Krause,
|
||||||
David Shaw, Norbert Frese, Bart Whiteley, Jean-Francois Bertrand, Ben Van Hof,
|
David Shaw, Norbert Frese, Bart Whiteley, Jean-Francois Bertrand, Ben Van Hof,
|
||||||
Yuriy Sosov, Christopher Palow, Yang Tse, Liam Healy, Nikolai Kondrashov,
|
Yuriy Sosov, Christopher Palow, Yang Tse, Liam Healy, Nikolai Kondrashov,
|
||||||
David Rosenstrauch, Andreas Faerber, Scott McCreary, Jeff Weber
|
David Rosenstrauch, Andreas Faerber, Scott McCreary, Jeff Weber, Emil Romanus
|
||||||
|
|
||||||
Thanks! (and sorry if I forgot to mention someone)
|
Thanks! (and sorry if I forgot to mention someone)
|
||||||
|
|
77
lib/multi.c
77
lib/multi.c
|
@ -189,8 +189,8 @@ static int update_timer(struct Curl_multi *multi);
|
||||||
static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle,
|
static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle,
|
||||||
struct connectdata *conn);
|
struct connectdata *conn);
|
||||||
static int checkPendPipeline(struct connectdata *conn);
|
static int checkPendPipeline(struct connectdata *conn);
|
||||||
static int moveHandleFromSendToRecvPipeline(struct SessionHandle *habdle,
|
static void moveHandleFromSendToRecvPipeline(struct SessionHandle *habdle,
|
||||||
struct connectdata *conn);
|
struct connectdata *conn);
|
||||||
static bool isHandleAtHead(struct SessionHandle *handle,
|
static bool isHandleAtHead(struct SessionHandle *handle,
|
||||||
struct curl_llist *pipeline);
|
struct curl_llist *pipeline);
|
||||||
|
|
||||||
|
@ -505,7 +505,7 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
|
||||||
sockets that time-out or have actions will be dealt with. Since this
|
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
|
handle has no action yet, we make sure it times out to get things to
|
||||||
happen. */
|
happen. */
|
||||||
Curl_expire(easy->easy_handle, 10);
|
Curl_expire(easy->easy_handle, 1);
|
||||||
|
|
||||||
/* increase the node-counter */
|
/* increase the node-counter */
|
||||||
multi->num_easy++;
|
multi->num_easy++;
|
||||||
|
@ -1286,13 +1286,19 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
|
||||||
/* call this even if the readwrite function returned error */
|
/* call this even if the readwrite function returned error */
|
||||||
Curl_posttransfer(easy->easy_handle);
|
Curl_posttransfer(easy->easy_handle);
|
||||||
|
|
||||||
|
/* we're no longer receving */
|
||||||
|
Curl_removeHandleFromPipeline(easy->easy_handle,
|
||||||
|
easy->easy_conn->recv_pipe);
|
||||||
|
|
||||||
|
/* expire the new receiving pipeline head */
|
||||||
|
if(easy->easy_conn->recv_pipe->head)
|
||||||
|
Curl_expire(easy->easy_conn->recv_pipe->head->ptr, 1);
|
||||||
|
|
||||||
|
/* Check if we can move pending requests to send pipe */
|
||||||
|
checkPendPipeline(easy->easy_conn);
|
||||||
|
|
||||||
/* When we follow redirects, must to go back to the CONNECT state */
|
/* When we follow redirects, must to go back to the CONNECT state */
|
||||||
if(easy->easy_handle->req.newurl || retry) {
|
if(easy->easy_handle->req.newurl || retry) {
|
||||||
Curl_removeHandleFromPipeline(easy->easy_handle,
|
|
||||||
easy->easy_conn->recv_pipe);
|
|
||||||
/* Check if we can move pending requests to send pipe */
|
|
||||||
checkPendPipeline(easy->easy_conn);
|
|
||||||
|
|
||||||
if(!retry) {
|
if(!retry) {
|
||||||
/* if the URL is a follow-location and not just a retried request
|
/* if the URL is a follow-location and not just a retried request
|
||||||
then figure out the URL here */
|
then figure out the URL here */
|
||||||
|
@ -1331,10 +1337,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
|
||||||
checkPendPipeline(easy->easy_conn);
|
checkPendPipeline(easy->easy_conn);
|
||||||
|
|
||||||
if(easy->easy_conn->bits.stream_was_rewound) {
|
if(easy->easy_conn->bits.stream_was_rewound) {
|
||||||
/* This request read past its response boundary so we quickly
|
/* This request read past its response boundary so we quickly let the
|
||||||
let the other requests consume those bytes since there is no
|
other requests consume those bytes since there is no guarantee that
|
||||||
guarantee that the socket will become active again */
|
the socket will become active again */
|
||||||
result = CURLM_CALL_MULTI_PERFORM;
|
result = CURLM_CALL_MULTI_PERFORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* post-transfer command */
|
/* post-transfer command */
|
||||||
|
@ -1431,7 +1437,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
|
||||||
get things to happen. Also, this makes it less important for callers of
|
get things to happen. Also, this makes it less important for callers of
|
||||||
the curl_multi_* functions to bother about the CURLM_CALL_MULTI_PERFORM
|
the curl_multi_* functions to bother about the CURLM_CALL_MULTI_PERFORM
|
||||||
return code, as long as they deal with the timeouts properly. */
|
return code, as long as they deal with the timeouts properly. */
|
||||||
Curl_expire(easy->easy_handle, 10);
|
Curl_expire(easy->easy_handle, 1);
|
||||||
|
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
@ -1785,6 +1791,9 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
|
||||||
extracts a matching node if there is one */
|
extracts a matching node if there is one */
|
||||||
|
|
||||||
now = Curl_tvnow();
|
now = Curl_tvnow();
|
||||||
|
now.tv_usec += 1000; /* to compensate for the truncating of 999us to 0ms,
|
||||||
|
we always add time here to make the comparison
|
||||||
|
below better */
|
||||||
|
|
||||||
multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
|
multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
|
||||||
if(t) {
|
if(t) {
|
||||||
|
@ -1962,6 +1971,7 @@ static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle,
|
||||||
static int checkPendPipeline(struct connectdata *conn)
|
static int checkPendPipeline(struct connectdata *conn)
|
||||||
{
|
{
|
||||||
int result = 0;
|
int result = 0;
|
||||||
|
struct curl_llist_element *sendhead = conn->send_pipe->head;
|
||||||
|
|
||||||
if (conn->server_supports_pipelining) {
|
if (conn->server_supports_pipelining) {
|
||||||
size_t pipeLen = conn->send_pipe->size + conn->recv_pipe->size;
|
size_t pipeLen = conn->send_pipe->size + conn->recv_pipe->size;
|
||||||
|
@ -1979,10 +1989,27 @@ static int checkPendPipeline(struct connectdata *conn)
|
||||||
conn->now = Curl_tvnow();
|
conn->now = Curl_tvnow();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if(result) {
|
||||||
|
/* something moved, check for a new send pipeline leader */
|
||||||
|
if(sendhead != conn->send_pipe->head) {
|
||||||
|
/* this is a new one as head, expire it */
|
||||||
|
conn->writechannel_inuse = FALSE; /* not in use yet */
|
||||||
|
infof(conn->data, "%p is at send pipe head!\n",
|
||||||
|
conn->send_pipe->head->ptr);
|
||||||
|
Curl_expire(conn->send_pipe->head->ptr, 1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
static int moveHandleFromSendToRecvPipeline(struct SessionHandle *handle,
|
/* Move this transfer from the sending list to the receiving list.
|
||||||
|
|
||||||
|
Pay special attention to the new sending list "leader" as it needs to get
|
||||||
|
checked to update what sockets it acts on.
|
||||||
|
|
||||||
|
*/
|
||||||
|
static void moveHandleFromSendToRecvPipeline(struct SessionHandle *handle,
|
||||||
struct connectdata *conn)
|
struct connectdata *conn)
|
||||||
{
|
{
|
||||||
struct curl_llist_element *curr;
|
struct curl_llist_element *curr;
|
||||||
|
@ -1992,12 +2019,24 @@ static int moveHandleFromSendToRecvPipeline(struct SessionHandle *handle,
|
||||||
if(curr->ptr == handle) {
|
if(curr->ptr == handle) {
|
||||||
Curl_llist_move(conn->send_pipe, curr,
|
Curl_llist_move(conn->send_pipe, curr,
|
||||||
conn->recv_pipe, conn->recv_pipe->tail);
|
conn->recv_pipe, conn->recv_pipe->tail);
|
||||||
return 1; /* we moved a handle */
|
|
||||||
|
if(conn->send_pipe->head) {
|
||||||
|
/* Since there's a new easy handle at the start of the send pipeline,
|
||||||
|
set its timeout value to 1ms to make it trigger instantly */
|
||||||
|
conn->writechannel_inuse = FALSE; /* not used now */
|
||||||
|
infof(conn->data, "%p is at send pipe head B!\n",
|
||||||
|
conn->send_pipe->head->ptr);
|
||||||
|
Curl_expire(conn->send_pipe->head->ptr, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* The receiver's list is not really interesting here since either this
|
||||||
|
handle is now first in the list and we'll deal with it soon, or
|
||||||
|
another handle is already first and thus is already taken care of */
|
||||||
|
|
||||||
|
break; /* we're done! */
|
||||||
}
|
}
|
||||||
curr = curr->next;
|
curr = curr->next;
|
||||||
}
|
}
|
||||||
|
|
||||||
return 0;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool isHandleAtHead(struct SessionHandle *handle,
|
static bool isHandleAtHead(struct SessionHandle *handle,
|
||||||
|
@ -2073,8 +2112,8 @@ void Curl_expire(struct SessionHandle *data, long milli)
|
||||||
|
|
||||||
*nowp = set;
|
*nowp = set;
|
||||||
#if 0
|
#if 0
|
||||||
infof(data, "Expire at %ld / %ld (%ldms)\n",
|
infof(data, "Expire at %ld / %ld (%ldms) %p\n",
|
||||||
(long)nowp->tv_sec, (long)nowp->tv_usec, milli);
|
(long)nowp->tv_sec, (long)nowp->tv_usec, milli, data);
|
||||||
#endif
|
#endif
|
||||||
data->state.timenode.payload = data;
|
data->state.timenode.payload = data;
|
||||||
multi->timetree = Curl_splayinsert(*nowp,
|
multi->timetree = Curl_splayinsert(*nowp,
|
||||||
|
|
Loading…
Reference in New Issue