- 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.
diff --git a/CHANGES b/CHANGES
index 7e7f997..6ff59e0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -8,6 +8,11 @@
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
provided excellent repeat recipes. I fixed the cases I managed to reproduce
but Jeff still got some (SCP) problems even after these fixes:
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 7beb1d4..117a6f8 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -39,6 +39,7 @@
o builds fine for Haiku OS
o follow redirect with only a new query string
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:
@@ -61,6 +62,6 @@
Rafa Muyo, Andre Guibert de Bruet, Brock Noland, Sandor Feldi, Stefan Krause,
David Shaw, Norbert Frese, Bart Whiteley, Jean-Francois Bertrand, Ben Van Hof,
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)
diff --git a/lib/multi.c b/lib/multi.c
index 3b6e4ae..9349ee6 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -189,8 +189,8 @@
static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle,
struct connectdata *conn);
static int checkPendPipeline(struct connectdata *conn);
-static int moveHandleFromSendToRecvPipeline(struct SessionHandle *habdle,
- struct connectdata *conn);
+static void moveHandleFromSendToRecvPipeline(struct SessionHandle *habdle,
+ struct connectdata *conn);
static bool isHandleAtHead(struct SessionHandle *handle,
struct curl_llist *pipeline);
@@ -505,7 +505,7 @@
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(easy->easy_handle, 10);
+ Curl_expire(easy->easy_handle, 1);
/* increase the node-counter */
multi->num_easy++;
@@ -1286,13 +1286,19 @@
/* call this even if the readwrite function returned error */
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 */
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 the URL is a follow-location and not just a retried request
then figure out the URL here */
@@ -1331,10 +1337,10 @@
checkPendPipeline(easy->easy_conn);
if(easy->easy_conn->bits.stream_was_rewound) {
- /* This request read past its response boundary so we quickly
- let the other requests consume those bytes since there is no
- guarantee that the socket will become active again */
- result = CURLM_CALL_MULTI_PERFORM;
+ /* This request read past its response boundary so we quickly let the
+ other requests consume those bytes since there is no guarantee that
+ the socket will become active again */
+ result = CURLM_CALL_MULTI_PERFORM;
}
/* post-transfer command */
@@ -1431,7 +1437,7 @@
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
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;
}
@@ -1785,6 +1791,9 @@
extracts a matching node if there is one */
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);
if(t) {
@@ -1962,6 +1971,7 @@
static int checkPendPipeline(struct connectdata *conn)
{
int result = 0;
+ struct curl_llist_element *sendhead = conn->send_pipe->head;
if (conn->server_supports_pipelining) {
size_t pipeLen = conn->send_pipe->size + conn->recv_pipe->size;
@@ -1979,10 +1989,27 @@
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;
}
-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 curl_llist_element *curr;
@@ -1992,12 +2019,24 @@
if(curr->ptr == handle) {
Curl_llist_move(conn->send_pipe, curr,
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;
}
-
- return 0;
}
static bool isHandleAtHead(struct SessionHandle *handle,
@@ -2073,8 +2112,8 @@
*nowp = set;
#if 0
- infof(data, "Expire at %ld / %ld (%ldms)\n",
- (long)nowp->tv_sec, (long)nowp->tv_usec, milli);
+ infof(data, "Expire at %ld / %ld (%ldms) %p\n",
+ (long)nowp->tv_sec, (long)nowp->tv_usec, milli, data);
#endif
data->state.timenode.payload = data;
multi->timetree = Curl_splayinsert(*nowp,
