Ravi Pratap provided a major update with pipelining fixes. We also no longer

re-use connections (for pipelining) before the name resolving is done.

6 files changed, 104 insertions, 60 deletions

diff --git a/CHANGES b/CHANGES
index 75affc1d2..6199e9c8e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,10 @@
 
                                   Changelog
 
+Daniel (23 October 2006)
+- Ravi Pratap provided a major update with pipelining fixes. We also no longer
+  re-use connections (for pipelining) before the name resolving is done.
+
 Daniel (21 October 2006)
 - Nir Soffer made the tests/libtest/Makefile.am use a proper variable for all
   the single test applications' link and dependences, so that you easier can
diff --git a/lib/multi.c b/lib/multi.c
index 6daabd00b..a7cc25562 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -773,7 +773,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       return CURLM_BAD_EASY_HANDLE;
 
     if (easy->easy_handle->state.pipe_broke) {
-      infof(easy->easy_handle, "Pipe broke: handle 0x%x\n", easy);
+      infof(easy->easy_handle, "Pipe broke: handle 0x%x, url = %s\n",
+            easy, easy->easy_handle->reqdata.path);
       if(easy->easy_handle->state.is_in_pipeline) {
         /* Head back to the CONNECT state */
         multistate(easy, CURLM_STATE_CONNECT);
diff --git a/lib/sendf.c b/lib/sendf.c
index 934fad1bb..01a23e445 100644
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -438,14 +438,25 @@ CURLcode Curl_client_write(struct connectdata *conn,
   return CURLE_OK;
 }
 
+#define MIN(a,b) (a < b ? a : b)
+
 void Curl_read_rewind(struct connectdata *conn,
                       size_t extraBytesRead)
 {
+    char buf[512 + 1];
+    size_t bytesToShow;
+
     conn->read_pos -= extraBytesRead;
     conn->bits.stream_was_rewound = TRUE;
-}
 
-#define MIN(a,b) (a < b ? a : b)
+    bytesToShow = MIN(conn->buf_len - conn->read_pos, sizeof(buf)-1);
+    memcpy(buf, conn->master_buffer + conn->read_pos, bytesToShow);
+    buf[bytesToShow] = '\0';
+
+    DEBUGF(infof(conn->data,
+                 "Buffer after stream rewind (read_pos = %d): [%s]",
+                 conn->read_pos, buf));
+}
 
 /*
  * Internal read-from-socket function. This is meant to deal with plain
@@ -457,12 +468,12 @@ void Curl_read_rewind(struct connectdata *conn,
 int Curl_read(struct connectdata *conn, /* connection data */
               curl_socket_t sockfd,     /* read from this socket */
               char *buf,                /* store read data here */
-              size_t buffersize,        /* max amount to read */
+              size_t sizerequested,     /* max amount to read */
               ssize_t *n)               /* amount bytes read */
 {
   ssize_t nread;
-  size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, buffersize);
-  size_t bytesremaining = buffersize - bytestocopy;
+  size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, sizerequested);
+  size_t bytesfromsocket = 0;
 
   /* Set 'num' to 0 or 1, depending on which socket that has been sent here.
      If it is the second socket, we set num to 1. Otherwise to 0. This lets
@@ -471,34 +482,34 @@ int Curl_read(struct connectdata *conn, /* connection data */
 
   *n=0; /* reset amount to zero */
 
-  bytesremaining = MIN(bytesremaining, sizeof(conn->master_buffer));
-
-  /* Copy from our master buffer first */
-  memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy);
-  conn->read_pos += bytestocopy;
+  /* Copy from our master buffer first if we have some unread data there*/
+  if (bytestocopy > 0) {
+    memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy);
+    conn->read_pos += bytestocopy;
+    conn->bits.stream_was_rewound = FALSE;
 
-  conn->bits.stream_was_rewound = FALSE;
-
-  *n = (ssize_t)bytestocopy;
-
-  if (bytesremaining == 0) {
-      return CURLE_OK;
+    *n = (ssize_t)bytestocopy;
+    return CURLE_OK;
   }
 
+  /* If we come here, it means that there is no data to read from the buffer,
+   * so we read from the socket */
+  bytesfromsocket = MIN(sizerequested, sizeof(conn->master_buffer));
+
   if(conn->ssl[num].use) {
-    nread = Curl_ssl_recv(conn, num, conn->master_buffer, bytesremaining);
+    nread = Curl_ssl_recv(conn, num, conn->master_buffer, bytesfromsocket);
 
-    if(nread == -1 && bytestocopy == 0) {
+    if(nread == -1)
       return -1; /* -1 from Curl_ssl_recv() means EWOULDBLOCK */
-    }
-
-  } else {
+  }
+  else {
     if(conn->sec_complete)
-      nread = Curl_sec_read(conn, sockfd, conn->master_buffer, bytesremaining);
+      nread = Curl_sec_read(conn, sockfd, conn->master_buffer,
+                            bytesfromsocket);
     else
-      nread = sread(sockfd, conn->master_buffer, bytesremaining);
+      nread = sread(sockfd, conn->master_buffer, bytesfromsocket);
 
-    if(-1 == nread && bytestocopy == 0) {
+    if(-1 == nread) {
       int err = Curl_sockerrno();
 #ifdef USE_WINSOCK
       if(WSAEWOULDBLOCK == err)
@@ -509,12 +520,12 @@ int Curl_read(struct connectdata *conn, /* connection data */
     }
   }
 
-  if (nread > 0) {
-      memcpy(buf, conn->master_buffer, nread);
+  if (nread >= 0) {
+    memcpy(buf, conn->master_buffer, nread);
 
-      conn->buf_len = nread;
-      conn->read_pos = nread;
-      *n += nread;
+    conn->buf_len = nread;
+    conn->read_pos = nread;
+    *n = nread;
   }
 
   return CURLE_OK;
diff --git a/lib/transfer.c b/lib/transfer.c
index e136020b0..300b8e9f6 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -743,8 +743,10 @@ CURLcode Curl_readwrite(struct connectdata *conn,
                 failf(data, "Maximum file size exceeded");
                 return CURLE_FILESIZE_EXCEEDED;
               }
-              if(contentlength >= 0)
+              if(contentlength >= 0) {
                 k->size = contentlength;
+                k->maxdownload = k->size;
+              }
               else {
                 /* Negative Content-Length is really odd, and we know it
                    happens for example when older Apache servers send large
@@ -1091,11 +1093,11 @@ CURLcode Curl_readwrite(struct connectdata *conn,
               Curl_debug(data, CURLINFO_DATA_IN, data->state.headerbuff,
                          (size_t)k->hbuflen, conn);
               if(k->badheader == HEADER_PARTHEADER)
-                Curl_debug(data, CURLINFO_DATA_IN, 
+                Curl_debug(data, CURLINFO_DATA_IN,
                            k->str, (size_t)nread, conn);
             }
             else
-              Curl_debug(data, CURLINFO_DATA_IN, 
+              Curl_debug(data, CURLINFO_DATA_IN,
                          k->str, (size_t)nread, conn);
           }
 
@@ -1133,13 +1135,17 @@ CURLcode Curl_readwrite(struct connectdata *conn,
 
           if((-1 != k->maxdownload) &&
              (k->bytecount + nread >= k->maxdownload)) {
-            size_t excess = (size_t)(k->bytecount + 
-                             (curl_off_t)nread - k->maxdownload);
-
-            if (excess > 0) {
-                infof(data, "Rewinding stream by : %d bytes\n", excess);
-                Curl_read_rewind(conn, excess);
-                conn->bits.stream_was_rewound = TRUE;
+            curl_off_t excess = k->bytecount +
+              ((curl_off_t) nread) - k->maxdownload;
+            if (excess > 0 && !k->ignorebody) {
+              infof(data,
+                    "Rewinding stream by : %" FORMAT_OFF_T
+                    " bytes on url %s (size = %" FORMAT_OFF_T
+                    ", maxdownload = %" FORMAT_OFF_T
+                    ", bytecount = %" FORMAT_OFF_T ", nread = %d)\n",
+                    excess, conn->data->reqdata.path,
+                    k->size, k->maxdownload, k->bytecount, nread);
+              Curl_read_rewind(conn, excess);
             }
 
             nread = (ssize_t) (k->maxdownload - k->bytecount);
diff --git a/lib/url.c b/lib/url.c
index 65453155c..4ca5d56a7 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -1854,6 +1854,7 @@ int Curl_removeHandleFromPipeline(struct SessionHandle *handle,
     }
     curr = curr->next;
   }
+
   return 0;
 }
 
@@ -1899,8 +1900,8 @@ static void signalPipeClose(struct curl_llist *pipe)
     }
     else
 #endif
-      data->state.pipe_broke = TRUE;
 
+    data->state.pipe_broke = TRUE;
     Curl_llist_remove(pipe, curr, NULL);
     curr = next;
   }
@@ -1936,12 +1937,19 @@ ConnectionExists(struct SessionHandle *data,
       /* NULL pointer means not filled-in entry */
       continue;
 
+#ifdef USE_ARES
+    /* ip_addr_str is NULL only if the resolving of the name hasn't completed
+       yet and until then we don't re-use this connection */
+    if (!check->ip_addr_str)
+        continue;
+#endif
+
     if(check->inuse && !canPipeline)
       /* can only happen within multi handles, and means that another easy
          handle is using this connection */
       continue;
 
-    if (check->send_pipe->size >= MAX_PIPELINE_LENGTH ||
+    if (check->send_pipe->size +
         check->recv_pipe->size >= MAX_PIPELINE_LENGTH)
       continue;
 
@@ -1994,14 +2002,28 @@ ConnectionExists(struct SessionHandle *data,
     }
 
     if(match) {
+
       bool dead = SocketIsDead(check->sock[FIRSTSOCKET]);
       if(dead) {
-        /*
-         */
-        check->data = data;
-        infof(data, "Connection %d seems to be dead!\n", i);
-        Curl_disconnect(check); /* disconnect resources */
-        data->state.connc->connects[i]=NULL; /* nothing here */
+        if (!check->is_in_pipeline) {
+          check->data = data;
+          infof(data, "Connection %d seems to be dead!\n", i);
+
+          Curl_disconnect(check); /* disconnect resources */
+          data->state.connc->connects[i]=NULL; /* nothing here */
+        }
+        else {
+          /* In the pipelining case, freeing the connection right away
+           * doesn't work. Instead, we mark the connection for close
+           * so that the handles will detect this automatically when
+           * they try to do something and deal with it there.
+           * Prematurely freeing this connection prevents handles that
+           * have already finished with their transfers to shut down
+           * cleanly.
+           */
+          infof(data, "Connection %d seems dead - marking for close\n", i);
+          check->bits.close = TRUE;
+        }
 
         /* There's no need to continue searching, because we only store
            one connection for each unique set of identifiers */
@@ -2012,10 +2034,13 @@ ConnectionExists(struct SessionHandle *data,
                               handle in a multi stack may nick it */
 
       if (canPipeline) {
-          /* Mark the connection as being in a pipeline */
-          check->is_in_pipeline = TRUE;
+        /* Mark the connection as being in a pipeline */
+        check->is_in_pipeline = TRUE;
       }
 
+      check->connectindex = i; /* Set this appropriately since it might have
+                                  been set to -1 when the easy was removed
+                                  from the multi */
       *usethis = check;
       return TRUE; /* yes, we found one to use! */
     }
@@ -2690,6 +2715,9 @@ static CURLcode CreateConnection(struct SessionHandle *data,
   conn->readchannel_inuse = FALSE;
   conn->writechannel_inuse = FALSE;
 
+  conn->read_pos = 0;
+  conn->buf_len = 0;
+
   /* Initialize the pipeline lists */
   conn->send_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor);
   conn->recv_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor);
@@ -3622,12 +3650,6 @@ static CURLcode CreateConnection(struct SessionHandle *data,
     infof(data, "Re-using existing connection! (#%ld) with host %s\n",
           conn->connectindex,
           conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname);
-#ifdef CURLRES_ASYNCH
-    if(!conn->ip_addr_str) {
-      infof(data, "... but it is not resolved yet!\n");
-      *async = TRUE;
-    }
-#endif
   }
   else {
     /*
@@ -3990,10 +4012,10 @@ CURLcode Curl_done(struct connectdata **connp,
 
   if(Curl_removeHandleFromPipeline(data, conn->recv_pipe) &&
      conn->readchannel_inuse)
-    conn->readchannel_inuse--;
+    conn->readchannel_inuse = FALSE;
   if(Curl_removeHandleFromPipeline(data, conn->send_pipe) &&
      conn->writechannel_inuse)
-    conn->writechannel_inuse--;
+    conn->writechannel_inuse = FALSE;
 
   /* cleanups done even if the connection is re-used */
   if(data->reqdata.rangestringalloc) {
diff --git a/lib/urldata.h b/lib/urldata.h
index c1b52dbf7..c93b46dff 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -812,8 +812,8 @@ struct connectdata {
                                    their responses on this pipeline */
 
   char master_buffer[BUFSIZE]; /* The master buffer for this connection. */
-  size_t read_pos;
-  size_t buf_len;
+  size_t read_pos; /* Current read position in the master buffer */
+  size_t buf_len; /* Length of the buffer?? */
 
 
   /*************** Request - specific items ************/