- 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 (file)
--- 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 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -39,6 +39,7 @@ This release includes the following bugfixes:
  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 @@ advice from friends like these:
  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 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -189,8 +189,8 @@ static int update_timer(struct Curl_multi *multi);
 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 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
      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 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         /* 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 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       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 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
        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 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
        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 CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle,
 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 @@ static int checkPendPipeline(struct connectdata *conn)
       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 @@ static int moveHandleFromSendToRecvPipeline(struct SessionHandle *handle,
     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 @@ void Curl_expire(struct SessionHandle *data, long milli)
 
     *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,