- Patrik Thunstrom reported a problem and helped me repeat it. It turned out

  libcurl did a superfluous 1000ms wait when doing SFTP downloads!

  We read data with libssh2 while doing the "DO" operation for SFTP and then
  when we were about to start getting data for the actual file part, the
  "TRANSFER" part, we waited for socket action (in 1000ms) before doing a
  libssh2-read. But in this case libssh2 had already read and buffered the
  data so we ended up always just waiting 1000ms before we get working on the
  data!

diff --git a/CHANGES b/CHANGES
index 6df1e73..9f30f0d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,17 @@
 
                                   Changelog
 
+Daniel Stenberg (19 Feb 2009)
+- Patrik Thunstrom reported a problem and helped me repeat it. It turned out 
+  libcurl did a superfluous 1000ms wait when doing SFTP downloads!
+
+  We read data with libssh2 while doing the "DO" operation for SFTP and then
+  when we were about to start getting data for the actual file part, the
+  "TRANSFER" part, we waited for socket action (in 1000ms) before doing a
+  libssh2-read. But in this case libssh2 had already read and buffered the
+  data so we ended up always just waiting 1000ms before we get working on the
+  data!
+
 Patrick Monnerat (18 Feb 2009)
 - FTP downloads (i.e.: RETR) ending with code 550 now return error
   CURLE_REMOTE_FILE_NOT_FOUND instead of CURLE_FTP_COULDNT_RETR_FILE.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index f9d6ca3..d6da342 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -35,6 +35,7 @@ This release includes the following bugfixes:
    CURLOPT_NOBODY set true
  o memory leak on some libz errors for content encodings
  o NSS-enabled build is repaired
+ o superfluous wait in SFTP downloads removed
 
 This release includes the following known bugs:
 
@@ -46,6 +47,6 @@ advice from friends like these:
  Lisa Xu, Daniel Fandrich, Craig A West, Alexey Borzov, Sharad Gupta,
  Peter Sylvester, Chad Monroe, Markus Moeller, Yang Tse, Scott Cantor,
  Patrick Scott, Hidemoto Nakada, Jocelyn Jaubert, Andre Guibert de Bruet,
- Kamil Dudka
+ Kamil Dudka, Patrik Thunstrom
 
         Thanks! (and sorry if I forgot to mention someone)

diff --git a/lib/transfer.c b/lib/transfer.c
index 17e3613..fe25ecc 100644 (file)
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1804,6 +1804,8 @@ Transfer(struct connectdata *conn)
   struct SessionHandle *data = conn->data;
   struct SingleRequest *k = &data->req;
   bool done=FALSE;
+  bool first=TRUE;
+  int timeout_ms;
 
   if((conn->sockfd == CURL_SOCKET_BAD) &&
      (conn->writesockfd == CURL_SOCKET_BAD))
@@ -1855,9 +1857,21 @@ Transfer(struct connectdata *conn)
        be no traffic during the select interval, we still call
        Curl_readwrite() for the timeout case and if we limit transfer speed we
        must make sure that this function doesn't transfer anything while in
-       HOLD status. */
+       HOLD status.
+
+       The no timeout for the first round is for the protocols for which data
+       has already been slurped off the socket and thus waiting for action
+       won't work since it'll wait even though there is already data present
+       to work with. */
+    if(first &&
+       ((fd_read != CURL_SOCKET_BAD) || (fd_write != CURL_SOCKET_BAD)))
+      /* if this is the first lap and one of the file descriptors is fine
+         to work with, skip the timeout */
+      timeout_ms = 0;
+    else
+      timeout_ms = 1000;
 
-    switch (Curl_socket_ready(fd_read, fd_write, 1000)) {
+    switch (Curl_socket_ready(fd_read, fd_write, timeout_ms)) {
     case -1: /* select() error, stop reading */
 #ifdef EINTR
       /* The EINTR is not serious, and it seems you might get this more
@@ -1870,12 +1884,13 @@ Transfer(struct connectdata *conn)
     default: /* readable descriptors */
 
       result = Curl_readwrite(conn, &done);
+      /* "done" signals to us if the transfer(s) are ready */
       break;
     }
     if(result)
       return result;
 
-    /* "done" signals to us if the transfer(s) are ready */
+    first = FALSE; /* not the first lap anymore */
   }
 
   return CURLE_OK;