From dfdac61522c7d660f884ec7a663dedb2d69d16a8 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 20 Dec 2011 12:52:24 +0100 Subject: non-blocking active FTP: cleanup multi state usage Backpedaled out the funny double-change of state in the multi state machine by adding a new argument to the do_more() function to signal completion. This way it can remain in the DO_MORE state properly until done. Long term, the entire DO_MORE logic should be moved into the FTP code and be hidden from the multi code as the logic is only used for FTP. --- lib/ftp.c | 38 +++++++++++++++++++++++++++++--------- lib/multi.c | 43 +++++++++++++++++-------------------------- lib/url.c | 17 ++++++++++++++--- lib/url.h | 2 +- lib/urldata.h | 2 +- 5 files changed, 62 insertions(+), 40 deletions(-) (limited to 'lib') diff --git a/lib/ftp.c b/lib/ftp.c index 3271907ec..2c5a98216 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -134,7 +134,7 @@ 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, bool dead_connection); -static CURLcode ftp_nextconnect(struct connectdata *conn); +static CURLcode ftp_do_more(struct connectdata *conn, bool *completed); static CURLcode ftp_multi_statemach(struct connectdata *conn, bool *done); static int ftp_getsock(struct connectdata *conn, curl_socket_t *socks, int numsocks); @@ -173,7 +173,7 @@ const struct Curl_handler Curl_handler_ftp = { ftp_setup_connection, /* setup_connection */ ftp_do, /* do_it */ ftp_done, /* done */ - ftp_nextconnect, /* do_more */ + ftp_do_more, /* do_more */ ftp_connect, /* connect_it */ ftp_multi_statemach, /* connecting */ ftp_doing, /* doing */ @@ -200,7 +200,7 @@ const struct Curl_handler Curl_handler_ftps = { ftp_setup_connection, /* setup_connection */ ftp_do, /* do_it */ ftp_done, /* done */ - ftp_nextconnect, /* do_more */ + ftp_do_more, /* do_more */ ftp_connect, /* connect_it */ ftp_multi_statemach, /* connecting */ ftp_doing, /* doing */ @@ -2356,6 +2356,8 @@ static CURLcode ftp_state_stor_resp(struct connectdata *conn, if(!connected) { infof(data, "Data conn was not available immediately\n"); + /* as there's not necessarily an immediate action on the control + connection now, we halt the state machine */ state(conn, FTP_STOP); conn->bits.wait_data_conn = TRUE; } @@ -3615,22 +3617,33 @@ static CURLcode ftp_range(struct connectdata *conn) /* - * ftp_nextconnect() + * ftp_do_more() * * This function shall be called when the second FTP (data) connection is * connected. */ -static CURLcode ftp_nextconnect(struct connectdata *conn) +static CURLcode ftp_do_more(struct connectdata *conn, bool *complete) { struct SessionHandle *data=conn->data; struct ftp_conn *ftpc = &conn->proto.ftpc; CURLcode result = CURLE_OK; + bool connected = FALSE; /* the ftp struct is inited in ftp_connect() */ struct FTP *ftp = data->state.proto.ftp; - DEBUGF(infof(data, "DO-MORE phase starts\n")); + /* if the second connection isn't done yet, wait for it */ + if(!conn->bits.tcpconnect[SECONDARYSOCKET]) { + result = Curl_is_connected(conn, SECONDARYSOCKET, &connected); + + /* Ready to do more? */ + if(connected) { + DEBUGF(infof(data, "DO-MORE connected phase starts\n")); + } + else + return result; + } if(ftp->transfer <= FTPTRANSFER_INFO) { /* a transfer is about to take place, or if not a file name was given @@ -3692,7 +3705,11 @@ static CURLcode ftp_nextconnect(struct connectdata *conn) too! */ Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL); - DEBUGF(infof(data, "DO-MORE phase ends with %d\n", (int)result)); + if(!conn->bits.wait_data_conn) { + /* no waiting for the data connection so this is now complete */ + *complete = TRUE; + DEBUGF(infof(data, "DO-MORE phase ends with %d\n", (int)result)); + } return result; } @@ -3974,6 +3991,7 @@ static CURLcode ftp_do(struct connectdata *conn, bool *done) CURLcode retcode = CURLE_OK; *done = FALSE; /* default to false */ + conn->bits.wait_data_conn = FALSE; /* default to no such wait */ /* Since connections can be re-used between SessionHandles, this might be a @@ -4343,8 +4361,10 @@ static CURLcode ftp_dophase_done(struct connectdata *conn, struct FTP *ftp = conn->data->state.proto.ftp; struct ftp_conn *ftpc = &conn->proto.ftpc; - if(connected) - result = ftp_nextconnect(conn); + if(connected) { + bool completed; + result = ftp_do_more(conn, &completed); + } if(result && (conn->sock[SECONDARYSOCKET] != CURL_SOCKET_BAD)) { /* Failure detected, close the second socket if it was created already */ diff --git a/lib/multi.c b/lib/multi.c index e408ab184..6ec20ec80 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1361,40 +1361,31 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, break; case CURLM_STATE_DO_MORE: - /* Ready to do more? */ - easy->result = Curl_is_connected(easy->easy_conn, - SECONDARYSOCKET, - &connected); - if(connected) { - /* - * When we are connected, DO MORE and then go DO_DONE - */ - easy->result = Curl_do_more(easy->easy_conn); - - /* No need to remove ourselves from the send pipeline here since that - is done for us in Curl_done() */ + /* + * When we are connected, DO MORE and then go DO_DONE + */ + easy->result = Curl_do_more(easy->easy_conn, &dophase_done); - if(CURLE_OK == easy->result) { + /* No need to remove this handle from the send pipeline here since that + is done in Curl_done() */ + if(CURLE_OK == easy->result) { + if(dophase_done) { multistate(easy, CURLM_STATE_DO_DONE); result = CURLM_CALL_MULTI_PERFORM; } - else { - /* failure detected */ - Curl_posttransfer(data); - Curl_done(&easy->easy_conn, easy->result, FALSE); - disconnect_conn = TRUE; - } + else + /* stay in DO_MORE */ + result = CURLM_OK; + } + else { + /* failure detected */ + Curl_posttransfer(data); + Curl_done(&easy->easy_conn, easy->result, FALSE); + disconnect_conn = TRUE; } break; case CURLM_STATE_DO_DONE: - - if(easy->easy_conn->bits.wait_data_conn == TRUE) { - multistate(easy, CURLM_STATE_DO_MORE); - result = CURLM_OK; - break; - } - /* Move ourselves from the send to recv pipeline */ moveHandleFromSendToRecvPipeline(data, easy->easy_conn); /* Check if we can move pending requests to send pipe */ diff --git a/lib/url.c b/lib/url.c index b952e920a..9896dd8c0 100644 --- a/lib/url.c +++ b/lib/url.c @@ -5457,14 +5457,25 @@ CURLcode Curl_do(struct connectdata **connp, bool *done) return result; } -CURLcode Curl_do_more(struct connectdata *conn) +/* + * Curl_do_more() is called during the DO_MORE multi state. It is basically a + * second stage DO state which (wrongly) was introduced to support FTP's + * second connection. + * + * TODO: A future libcurl should be able to work away this state. + * + */ + +CURLcode Curl_do_more(struct connectdata *conn, bool *completed) { CURLcode result=CURLE_OK; + *completed = FALSE; + if(conn->handler->do_more) - result = conn->handler->do_more(conn); + result = conn->handler->do_more(conn, completed); - if(result == CURLE_OK && conn->bits.wait_data_conn == FALSE) + if(!result && completed) /* do_complete must be called after the protocol-specific DO function */ do_complete(conn); diff --git a/lib/url.h b/lib/url.h index 8947627d5..c858706a1 100644 --- a/lib/url.h +++ b/lib/url.h @@ -37,7 +37,7 @@ CURLcode Curl_close(struct SessionHandle *data); /* opposite of curl_open() */ CURLcode Curl_connect(struct SessionHandle *, struct connectdata **, bool *async, bool *protocol_connect); CURLcode Curl_do(struct connectdata **, bool *done); -CURLcode Curl_do_more(struct connectdata *); +CURLcode Curl_do_more(struct connectdata *, bool *completed); CURLcode Curl_done(struct connectdata **, CURLcode, bool premature); CURLcode Curl_disconnect(struct connectdata *, bool dead_connection); CURLcode Curl_protocol_connect(struct connectdata *conn, bool *done); diff --git a/lib/urldata.h b/lib/urldata.h index f7c35e367..822412d05 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -512,7 +512,7 @@ struct Curl_async { /* These function pointer types are here only to allow easier typecasting within the source when we need to cast between data pointers (such as NULL) and function pointers. */ -typedef CURLcode (*Curl_do_more_func)(struct connectdata *); +typedef CURLcode (*Curl_do_more_func)(struct connectdata *, bool *); typedef CURLcode (*Curl_done_func)(struct connectdata *, CURLcode, bool); -- cgit v1.2.1