Hello, attached is a more generic version of the patch for rhbz #650255 [1] we discussed on IRC the other day. It affects FTP, POP3, IMAP and SMTP for now. I tested it only with FTP, using the follwoing test-case:
https://bugzilla.redhat.com/attachment.cgi?id=460111 It seems to solve the problem without breaking anything else. Kamil [1] https://bugzilla.redhat.com/650255
From f6d3fef0f036e192bc775c1a603ed8a9317ac2a6 Mon Sep 17 00:00:00 2001 From: Kamil Dudka <[email protected]> Date: Fri, 19 Nov 2010 13:43:20 +0100 Subject: [PATCH] url: provide dead_connection flag in Curl_handler::disconnect It helps to prevent a hangup with some FTP servers in case idle session timeout has exceeded. But it may be useful also for other protocols that send any quit message on disconnect. Currently used by FTP, POP3, IMAP and SMTP. --- lib/ftp.c | 6 ++++-- lib/imap.c | 6 +++--- lib/multi.c | 7 ++++--- lib/openldap.c | 5 +++-- lib/pop3.c | 6 +++--- lib/rtsp.c | 4 +++- lib/rtsp.h | 2 +- lib/smtp.c | 6 +++--- lib/ssh.c | 10 ++++++---- lib/tftp.c | 5 +++-- lib/transfer.c | 2 +- lib/url.c | 15 ++++++++------- lib/url.h | 2 +- lib/urldata.h | 6 ++++-- 14 files changed, 47 insertions(+), 35 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index dc3ab37..f8309bb 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -136,7 +136,7 @@ static CURLcode ftp_do(struct connectdata *conn, bool *done); static CURLcode ftp_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode ftp_connect(struct connectdata *conn, bool *done); -static CURLcode ftp_disconnect(struct connectdata *conn); +static CURLcode ftp_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode ftp_nextconnect(struct connectdata *conn); static CURLcode ftp_multi_statemach(struct connectdata *conn, bool *done); static int ftp_getsock(struct connectdata *conn, @@ -3842,7 +3842,7 @@ static CURLcode ftp_quit(struct connectdata *conn) * Disconnect from an FTP server. Cleanup protocol-specific per-connection * resources. BLOCKING. */ -static CURLcode ftp_disconnect(struct connectdata *conn) +static CURLcode ftp_disconnect(struct connectdata *conn, bool dead_connection) { struct ftp_conn *ftpc= &conn->proto.ftpc; struct pingpong *pp = &ftpc->pp; @@ -3854,6 +3854,8 @@ static CURLcode ftp_disconnect(struct connectdata *conn) ftp_quit() will check the state of ftp->ctl_valid. If it's ok it will try to send the QUIT command, otherwise it will just return. */ + if(dead_connection) + ftpc->ctl_valid = FALSE; /* The FTP session may or may not have been allocated/setup at this point! */ (void)ftp_quit(conn); /* ignore errors on the QUIT */ diff --git a/lib/imap.c b/lib/imap.c index cdadd17..9c39625 100644 --- a/lib/imap.c +++ b/lib/imap.c @@ -100,7 +100,7 @@ static CURLcode imap_do(struct connectdata *conn, bool *done); static CURLcode imap_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode imap_connect(struct connectdata *conn, bool *done); -static CURLcode imap_disconnect(struct connectdata *conn); +static CURLcode imap_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode imap_multi_statemach(struct connectdata *conn, bool *done); static int imap_getsock(struct connectdata *conn, curl_socket_t *socks, @@ -877,13 +877,13 @@ static CURLcode imap_logout(struct connectdata *conn) * Disconnect from an IMAP server. Cleanup protocol-specific per-connection * resources. BLOCKING. */ -static CURLcode imap_disconnect(struct connectdata *conn) +static CURLcode imap_disconnect(struct connectdata *conn, bool dead_connection) { struct imap_conn *imapc= &conn->proto.imapc; /* The IMAP session may or may not have been allocated/setup at this point! */ - if (imapc->pp.conn) + if(!dead_connection && imapc->pp.conn) (void)imap_logout(conn); /* ignore errors on the LOGOUT */ Curl_pp_disconnect(&imapc->pp); diff --git a/lib/multi.c b/lib/multi.c index 875e136..0422eaa 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1632,7 +1632,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } if(disconnect_conn) { - Curl_disconnect(easy->easy_conn); /* disconnect properly */ + /* disconnect properly */ + Curl_disconnect(easy->easy_conn, /* dead_connection */ FALSE); /* This is where we make sure that the easy_conn pointer is reset. We don't have to do this in every case block above where a @@ -1757,7 +1758,7 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) for(i=0; i< multi->connc->num; i++) { if(multi->connc->connects[i] && multi->connc->connects[i]->protocol & PROT_CLOSEACTION) { - Curl_disconnect(multi->connc->connects[i]); + Curl_disconnect(multi->connc->connects[i], /* dead_connection */ FALSE); multi->connc->connects[i] = NULL; } } @@ -2666,7 +2667,7 @@ static void multi_connc_remove_handle(struct Curl_multi *multi, data->state.shared_conn = multi; else { /* out of memory - so much for graceful shutdown */ - Curl_disconnect(conn); + Curl_disconnect(conn, /* dead_connection */ FALSE); multi->connc->connects[i] = NULL; } } diff --git a/lib/openldap.c b/lib/openldap.c index ee4915f..17cbe00 100644 --- a/lib/openldap.c +++ b/lib/openldap.c @@ -61,7 +61,7 @@ static CURLcode ldap_do(struct connectdata *conn, bool *done); static CURLcode ldap_done(struct connectdata *conn, CURLcode, bool); static CURLcode ldap_connect(struct connectdata *conn, bool *done); static CURLcode ldap_connecting(struct connectdata *conn, bool *done); -static CURLcode ldap_disconnect(struct connectdata *conn); +static CURLcode ldap_disconnect(struct connectdata *conn, bool dead_connection); static Curl_recv ldap_recv; @@ -344,9 +344,10 @@ retry: return CURLE_OK; } -static CURLcode ldap_disconnect(struct connectdata *conn) +static CURLcode ldap_disconnect(struct connectdata *conn, bool dead_connection) { ldapconninfo *li = conn->proto.generic; + (void) dead_connection; if (li) { if (li->ld) { diff --git a/lib/pop3.c b/lib/pop3.c index 9609b62..9f67443 100644 --- a/lib/pop3.c +++ b/lib/pop3.c @@ -101,7 +101,7 @@ static CURLcode pop3_do(struct connectdata *conn, bool *done); static CURLcode pop3_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode pop3_connect(struct connectdata *conn, bool *done); -static CURLcode pop3_disconnect(struct connectdata *conn); +static CURLcode pop3_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode pop3_multi_statemach(struct connectdata *conn, bool *done); static int pop3_getsock(struct connectdata *conn, curl_socket_t *socks, @@ -817,7 +817,7 @@ static CURLcode pop3_quit(struct connectdata *conn) * Disconnect from an POP3 server. Cleanup protocol-specific per-connection * resources. BLOCKING. */ -static CURLcode pop3_disconnect(struct connectdata *conn) +static CURLcode pop3_disconnect(struct connectdata *conn, bool dead_connection) { struct pop3_conn *pop3c= &conn->proto.pop3c; @@ -828,7 +828,7 @@ static CURLcode pop3_disconnect(struct connectdata *conn) /* The POP3 session may or may not have been allocated/setup at this point! */ - if(pop3c->pp.conn) + if(!dead_connection && pop3c->pp.conn) (void)pop3_quit(conn); /* ignore errors on the LOGOUT */ diff --git a/lib/rtsp.c b/lib/rtsp.c index 1254c73..066e10f 100644 --- a/lib/rtsp.c +++ b/lib/rtsp.c @@ -117,7 +117,9 @@ CURLcode Curl_rtsp_connect(struct connectdata *conn, bool *done) return httpStatus; } -CURLcode Curl_rtsp_disconnect(struct connectdata *conn) { +CURLcode Curl_rtsp_disconnect(struct connectdata *conn, bool dead_connection) +{ + (void) dead_connection; Curl_safefree(conn->proto.rtspc.rtp_buf); return CURLE_OK; } diff --git a/lib/rtsp.h b/lib/rtsp.h index c3f8bd7..82e0706 100644 --- a/lib/rtsp.h +++ b/lib/rtsp.h @@ -43,7 +43,7 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data, CURLcode Curl_rtsp(struct connectdata *conn, bool *done); CURLcode Curl_rtsp_done(struct connectdata *conn, CURLcode, bool premature); CURLcode Curl_rtsp_connect(struct connectdata *conn, bool *done); -CURLcode Curl_rtsp_disconnect(struct connectdata *conn); +CURLcode Curl_rtsp_disconnect(struct connectdata *conn, bool dead_connection); CURLcode Curl_rtsp_parseheader(struct connectdata *conn, char *header); diff --git a/lib/smtp.c b/lib/smtp.c index 55e03d5..4fa8662 100644 --- a/lib/smtp.c +++ b/lib/smtp.c @@ -106,7 +106,7 @@ static CURLcode smtp_do(struct connectdata *conn, bool *done); static CURLcode smtp_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode smtp_connect(struct connectdata *conn, bool *done); -static CURLcode smtp_disconnect(struct connectdata *conn); +static CURLcode smtp_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode smtp_multi_statemach(struct connectdata *conn, bool *done); static int smtp_getsock(struct connectdata *conn, curl_socket_t *socks, @@ -1308,7 +1308,7 @@ static CURLcode smtp_quit(struct connectdata *conn) * Disconnect from an SMTP server. Cleanup protocol-specific per-connection * resources. BLOCKING. */ -static CURLcode smtp_disconnect(struct connectdata *conn) +static CURLcode smtp_disconnect(struct connectdata *conn, bool dead_connection) { struct smtp_conn *smtpc= &conn->proto.smtpc; @@ -1319,7 +1319,7 @@ static CURLcode smtp_disconnect(struct connectdata *conn) /* The SMTP session may or may not have been allocated/setup at this point! */ - if(smtpc->pp.conn) + if(!dead_connection && smtpc->pp.conn) (void)smtp_quit(conn); /* ignore errors on the LOGOUT */ Curl_pp_disconnect(&smtpc->pp); diff --git a/lib/ssh.c b/lib/ssh.c index 862ce76..25da541 100644 --- a/lib/ssh.c +++ b/lib/ssh.c @@ -134,13 +134,13 @@ static CURLcode scp_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode scp_doing(struct connectdata *conn, bool *dophase_done); -static CURLcode scp_disconnect(struct connectdata *conn); +static CURLcode scp_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode sftp_done(struct connectdata *conn, CURLcode, bool premature); static CURLcode sftp_doing(struct connectdata *conn, bool *dophase_done); -static CURLcode sftp_disconnect(struct connectdata *conn); +static CURLcode sftp_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode sftp_perform(struct connectdata *conn, bool *connected, @@ -2692,10 +2692,11 @@ static CURLcode ssh_do(struct connectdata *conn, bool *done) /* BLOCKING, but the function is using the state machine so the only reason this is still blocking is that the multi interface code has no support for disconnecting operations that takes a while */ -static CURLcode scp_disconnect(struct connectdata *conn) +static CURLcode scp_disconnect(struct connectdata *conn, bool dead_connection) { CURLcode result = CURLE_OK; struct ssh_conn *ssh = &conn->proto.sshc; + (void) dead_connection; Curl_safefree(conn->data->state.proto.ssh); conn->data->state.proto.ssh = NULL; @@ -2856,9 +2857,10 @@ static CURLcode sftp_doing(struct connectdata *conn, /* BLOCKING, but the function is using the state machine so the only reason this is still blocking is that the multi interface code has no support for disconnecting operations that takes a while */ -static CURLcode sftp_disconnect(struct connectdata *conn) +static CURLcode sftp_disconnect(struct connectdata *conn, bool dead_connection) { CURLcode result = CURLE_OK; + (void) dead_connection; DEBUGF(infof(conn->data, "SSH DISCONNECT starts now\n")); diff --git a/lib/tftp.c b/lib/tftp.c index 46ed2a7..fc741c9 100644 --- a/lib/tftp.c +++ b/lib/tftp.c @@ -166,7 +166,7 @@ typedef struct tftp_state_data { static CURLcode tftp_rx(tftp_state_data_t *state, tftp_event_t event) ; static CURLcode tftp_tx(tftp_state_data_t *state, tftp_event_t event) ; static CURLcode tftp_connect(struct connectdata *conn, bool *done); -static CURLcode tftp_disconnect(struct connectdata *conn); +static CURLcode tftp_disconnect(struct connectdata *conn, bool dead_connection); static CURLcode tftp_do(struct connectdata *conn, bool *done); static CURLcode tftp_done(struct connectdata *conn, CURLcode, bool premature); @@ -925,9 +925,10 @@ static CURLcode tftp_state_machine(tftp_state_data_t *state, * The disconnect callback * **********************************************************/ -static CURLcode tftp_disconnect(struct connectdata *conn) +static CURLcode tftp_disconnect(struct connectdata *conn, bool dead_connection) { tftp_state_data_t *state = conn->proto.tftpc; + (void) dead_connection; /* done, free dynamically allocated pkt buffers */ if(state) { diff --git a/lib/transfer.c b/lib/transfer.c index ead3a4d..e4e3405 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -1959,7 +1959,7 @@ connect_host(struct SessionHandle *data, res = Curl_async_resolved(*conn, &protocol_done); else /* if we can't resolve, we kill this "connection" now */ - (void)Curl_disconnect(*conn); + (void)Curl_disconnect(*conn, /* dead_connection */ FALSE); } return res; diff --git a/lib/url.c b/lib/url.c index e915c79..479e979 100644 --- a/lib/url.c +++ b/lib/url.c @@ -651,7 +651,7 @@ CURLcode Curl_ch_connc(struct SessionHandle *data, close handles not in use. */ for(i=newamount; i< c->num; i++) - Curl_disconnect(c->connects[i]); + Curl_disconnect(c->connects[i], /* dead_connection */ FALSE); /* If the most recent connection is no longer valid, mark it invalid. */ @@ -2573,7 +2573,7 @@ static void conn_free(struct connectdata *conn) free(conn); /* free all the connection oriented data */ } -CURLcode Curl_disconnect(struct connectdata *conn) +CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection) { struct SessionHandle *data; if(!conn) @@ -2633,7 +2633,7 @@ CURLcode Curl_disconnect(struct connectdata *conn) if(conn->handler->disconnect) /* This is set if protocol-specific cleanups should be made */ - conn->handler->disconnect(conn); + conn->handler->disconnect(conn, dead_connection); if(-1 != conn->connectindex) { /* unlink ourselves! */ @@ -2901,7 +2901,8 @@ ConnectionExists(struct SessionHandle *data, check->data = data; infof(data, "Connection #%ld seems to be dead!\n", i); - Curl_disconnect(check); /* disconnect resources */ + /* disconnect resources */ + Curl_disconnect(check, /* dead_connection */ TRUE); data->state.connc->connects[i]=NULL; /* nothing here */ continue; @@ -3088,7 +3089,7 @@ ConnectionKillOne(struct SessionHandle *data) conn->data = data; /* the winner gets the honour of being disconnected */ - (void)Curl_disconnect(conn); + (void)Curl_disconnect(conn, /* dead_connection */ FALSE); /* clean the array entry */ data->state.connc->connects[connindex] = NULL; @@ -5105,7 +5106,7 @@ CURLcode Curl_connect(struct SessionHandle *data, if(code && *in_connect) { /* We're not allowed to return failure with memory left allocated in the connectdata struct, free those here */ - Curl_disconnect(*in_connect); /* close the connection */ + Curl_disconnect(*in_connect, FALSE); /* close the connection */ *in_connect = NULL; /* return a NULL */ } @@ -5225,7 +5226,7 @@ CURLcode Curl_done(struct connectdata **connp, */ if(data->set.reuse_forbid || conn->bits.close || premature || (-1 == conn->connectindex)) { - CURLcode res2 = Curl_disconnect(conn); /* close the connection */ + CURLcode res2 = Curl_disconnect(conn, FALSE); /* close the connection */ /* If we had an error already, make sure we return that one. But if we got a new error, return that. */ diff --git a/lib/url.h b/lib/url.h index 63d7f2c..241dc28 100644 --- a/lib/url.h +++ b/lib/url.h @@ -42,7 +42,7 @@ CURLcode Curl_async_resolved(struct connectdata *conn, CURLcode Curl_do(struct connectdata **, bool *done); CURLcode Curl_do_more(struct connectdata *); CURLcode Curl_done(struct connectdata **, CURLcode, bool premature); -CURLcode Curl_disconnect(struct connectdata *); +CURLcode Curl_disconnect(struct connectdata *, bool dead_connection); CURLcode Curl_protocol_connect(struct connectdata *conn, bool *done); CURLcode Curl_protocol_connecting(struct connectdata *conn, bool *done); CURLcode Curl_protocol_doing(struct connectdata *conn, bool *done); diff --git a/lib/urldata.h b/lib/urldata.h index 93c2d40..93aefc2 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -646,9 +646,11 @@ struct Curl_handler { int numsocks); /* This function *MAY* be set to a protocol-dependent function that is run - * by the curl_disconnect(), as a step in the disconnection. + * by the curl_disconnect(), as a step in the disconnection. If the handler + * is called because the connection has been considered dead, dead_connection + * is set to TRUE. */ - CURLcode (*disconnect)(struct connectdata *); + CURLcode (*disconnect)(struct connectdata *, bool dead_connection); long defport; /* Default port. */ long protocol; /* PROT_* flags concerning the protocol set */ -- 1.7.3.2
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
