From 5af0629ba5d09b9820fe3752279906147c310e3e Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 19 Feb 2009 10:36:20 +0000 Subject: [PATCH] - 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! --- CHANGES | 11 +++++++++++ RELEASE-NOTES | 3 ++- lib/transfer.c | 21 ++++++++++++++++++--- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 6df1e738f..9f30f0dc4 100644 --- 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 f9d6ca3db..d6da34205 100644 --- 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 17e361366..fe25eccf5 100644 --- 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. - switch (Curl_socket_ready(fd_read, fd_write, 1000)) { + 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, 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;