diff --git a/CHANGES b/CHANGES index 9f30f0dc4..8c480ade9 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,16 @@ Changelog +Daniel Stenberg (20 Feb 2009) +- Linus Nielsen Feltzing reported and helped me repeat and fix a problem with + FTP with the multi interface: when a transfer fails, like when aborted by a + write callback, the control connection was wrongly closed and thus not + re-used properly. + + This change is also an attempt to cleanup the code somewhat in this area, as + now the FTP code attempts to keep (better) track on pending responses + necessary to get read in ftp_done(). + 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! diff --git a/RELEASE-NOTES b/RELEASE-NOTES index d6da34205..972157474 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -47,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, Patrik Thunstrom + Kamil Dudka, Patrik Thunstrom, Linus Nielsen Feltzing Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/ftp.c b/lib/ftp.c index cf2d4db6a..426f4af76 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -363,6 +363,7 @@ static void ftp_respinit(struct connectdata *conn) struct ftp_conn *ftpc = &conn->proto.ftpc; ftpc->nread_resp = 0; ftpc->linestart_resp = conn->data->state.buffer; + ftpc->pending_resp = TRUE; } /* macro to check for a three-digit ftp status code at the start of the @@ -590,6 +591,8 @@ static CURLcode ftp_readresp(curl_socket_t sockfd, /* store the latest code for later retrieval */ conn->data->info.httpcode=code; + ftpc->pending_resp = FALSE; + return result; } @@ -715,6 +718,8 @@ CURLcode Curl_GetFTPResponse(ssize_t *nreadp, /* return number of bytes read */ } /* while there's buffer left and loop is requested */ + ftpc->pending_resp = FALSE; + return result; } @@ -2305,6 +2310,8 @@ static CURLcode ftp_state_stor_resp(struct connectdata *conn, SECONDARYSOCKET, ftp->bytecountp); state(conn, FTP_STOP); + conn->proto.ftpc.pending_resp = TRUE; /* we expect a server response more */ + return result; } @@ -2417,6 +2424,7 @@ static CURLcode ftp_state_get_resp(struct connectdata *conn, if(result) return result; + conn->proto.ftpc.pending_resp = TRUE; /* we expect a server response more */ state(conn, FTP_STOP); } else { @@ -3161,6 +3169,7 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status, case CURLE_REMOTE_ACCESS_DENIED: case CURLE_FILESIZE_EXCEEDED: case CURLE_REMOTE_FILE_NOT_FOUND: + case CURLE_WRITE_ERROR: /* the connection stays alive fine even though this happened */ /* fall-through */ case CURLE_OK: /* doesn't affect the control connection's status */ @@ -3239,7 +3248,8 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status, conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD; } - if((ftp->transfer == FTPTRANSFER_BODY) && !status && !premature) { + if((ftp->transfer == FTPTRANSFER_BODY) && ftpc->ctl_valid && + ftpc->pending_resp && !premature) { /* * Let's see what the server says about the transfer we just performed, * but lower the timeout as sometimes this connection has died while the diff --git a/lib/multi.c b/lib/multi.c index 15339a3d6..2ce023331 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1237,14 +1237,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, break; case CURLM_STATE_WAITPERFORM: -#ifdef CURLDEBUG - infof(easy->easy_handle, "Conn %d recv pipe %d inuse %d athead %d\n", - easy->easy_conn->connectindex, - easy->easy_conn->recv_pipe->size, - easy->easy_conn->readchannel_inuse, - isHandleAtHead(easy->easy_handle, - easy->easy_conn->recv_pipe)); -#endif /* Wait for our turn to PERFORM */ if(!easy->easy_conn->readchannel_inuse && isHandleAtHead(easy->easy_handle, @@ -1254,6 +1246,16 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, multistate(easy, CURLM_STATE_PERFORM); result = CURLM_CALL_MULTI_PERFORM; } +#ifdef CURLDEBUG + else { + infof(easy->easy_handle, "Conn %d recv pipe %d inuse %d athead %d\n", + easy->easy_conn->connectindex, + easy->easy_conn->recv_pipe->size, + easy->easy_conn->readchannel_inuse, + isHandleAtHead(easy->easy_handle, + easy->easy_conn->recv_pipe)); + } +#endif break; case CURLM_STATE_TOOFAST: /* limit-rate exceeded in either direction */ @@ -1302,18 +1304,14 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, if(easy->result) { /* The transfer phase returned error, we mark the connection to get - * closed to prevent being re-used. This is because we can't - * possibly know if the connection is in a good shape or not now. */ - easy->easy_conn->bits.close = TRUE; - Curl_removeHandleFromPipeline(easy->easy_handle, - easy->easy_conn->recv_pipe); + * closed to prevent being re-used. This is because we can't possibly + * know if the connection is in a good shape or not now. Unless it is + * a protocol which uses two "channels" like FTP, as then the error + * happened in the data connection. + */ + if(!(easy->easy_conn->protocol & PROT_DUALCHANNEL)) + easy->easy_conn->bits.close = TRUE; - if(CURL_SOCKET_BAD != easy->easy_conn->sock[SECONDARYSOCKET]) { - /* if we failed anywhere, we must clean up the secondary socket if - it was used */ - sclose(easy->easy_conn->sock[SECONDARYSOCKET]); - easy->easy_conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD; - } Curl_posttransfer(easy->easy_handle); Curl_done(&easy->easy_conn, easy->result, FALSE); } diff --git a/lib/urldata.h b/lib/urldata.h index d1851066e..6969f1ebb 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -438,6 +438,9 @@ struct ftp_conn { size_t nread_resp; /* number of bytes currently read of a server response */ char *linestart_resp; /* line start pointer for the FTP server response reader function */ + bool pending_resp; /* set TRUE when a server response is pending or in + progress, and is cleared once the last response is + read */ int count1; /* general purpose counter for the state machine */ int count2; /* general purpose counter for the state machine */ @@ -904,6 +907,7 @@ struct connectdata { #define PROT_CLOSEACTION PROT_FTP /* these ones need action before socket close */ +#define PROT_DUALCHANNEL PROT_FTP /* these protocols use two connections */ /* 'dns_entry' is the particular host we use. This points to an entry in the DNS cache and it will not get pruned while locked. It gets unlocked in