diff --git a/CHANGES b/CHANGES index eca2dd0ad..3edd67c3e 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,8 @@ Changelog Yang Tse (10 April 2007) +- Ravi Pratap provided some fixes for HTTP pipelining + - configure script will ignore --enable-sspi option for non-native Windows. Daniel S (9 April 2007) diff --git a/lib/ftp.c b/lib/ftp.c index 4bc91b9f6..4007bb091 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -331,7 +331,7 @@ static CURLcode ftp_readresp(curl_socket_t sockfd, * line */ int i; - conn->headerbytecount += gotbytes; + data->reqdata.keep.headerbytecount += gotbytes; ftpc->nread_resp += gotbytes; for(i = 0; i < gotbytes; ptr++, i++) { @@ -564,7 +564,7 @@ CURLcode Curl_GetFTPResponse(ssize_t *nreadp, /* return number of bytes read */ * line */ int i; - conn->headerbytecount += gotbytes; + data->reqdata.keep.headerbytecount += gotbytes; *nreadp += gotbytes; for(i = 0; i < gotbytes; ptr++, i++) { diff --git a/lib/http.c b/lib/http.c index 8e4d322af..c412df5bb 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1630,8 +1630,8 @@ CURLcode Curl_http_done(struct connectdata *conn, if(!conn->bits.retry && ((http->readbytecount + - conn->headerbytecount - - conn->deductheadercount)) <= 0) { + data->reqdata.keep.headerbytecount - + data->reqdata.keep.deductheadercount)) <= 0) { /* If this connection isn't simply closed to be retried, AND nothing was read from the HTTP server (that counts), this can't be right so we return an error here */ diff --git a/lib/multi.c b/lib/multi.c index 17003f47b..3245b520d 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -207,7 +207,7 @@ void curl_multi_dump(CURLM *multi_handle); static void multistate(struct Curl_one_easy *easy, CURLMstate state) { #ifdef CURLDEBUG - long index = -1; + long index = -5000; #endif CURLMstate oldstate = easy->state; @@ -534,10 +534,12 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle, multi->num_alive--; if (easy->easy_handle->state.is_in_pipeline && - easy->state > CURLM_STATE_DO) { + easy->state > CURLM_STATE_DO && + easy->state < CURLM_STATE_COMPLETED) { /* If the handle is in a pipeline and has finished sending off its - request, we need to remember the fact that we want to remove this - handle but do the actual removal at a later time */ + request but not received its reponse yet, we need to remember the + fact that we want to remove this handle but do the actual removal at + a later time */ easy->easy_handle->state.cancelled = TRUE; return CURLM_OK; } @@ -603,7 +605,9 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle, /* and modify the connectindex since this handle can't point to the connection cache anymore */ - if(easy->easy_conn) + if(easy->easy_conn && + (easy->easy_conn->send_pipe->size + + easy->easy_conn->recv_pipe->size == 0)) easy->easy_conn->connectindex = -1; } @@ -648,6 +652,14 @@ bool Curl_multi_canPipeline(struct Curl_multi* multi) return multi->pipelining_enabled; } +void Curl_multi_handlePipeBreak(struct SessionHandle *data) +{ + struct Curl_one_easy *one_easy = data->set.one_easy; + + if (one_easy) + one_easy->easy_conn = NULL; +} + static int waitconnect_getsock(struct connectdata *conn, curl_socket_t *sock, int numsocks) @@ -791,13 +803,17 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, struct Curl_transfer_keeper *k; do { + bool disconnect_conn = FALSE; if(!GOOD_EASY_HANDLE(easy->easy_handle)) return CURLM_BAD_EASY_HANDLE; + /* Handle the case when the pipe breaks, i.e., the connection + we're using gets cleaned up and we're left with nothing. */ if (easy->easy_handle->state.pipe_broke) { infof(easy->easy_handle, "Pipe broke: handle 0x%x, url = %s\n", easy, easy->easy_handle->reqdata.path); + if(easy->easy_handle->state.is_in_pipeline) { /* Head back to the CONNECT state */ multistate(easy, CURLM_STATE_CONNECT); @@ -920,7 +936,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* call again please so that we get the next socket setup */ result = CURLM_CALL_MULTI_PERFORM; if(protocol_connect) - multistate(easy, CURLM_STATE_DO); + multistate(easy, CURLM_STATE_WAITDO); else { #ifndef CURL_DISABLE_HTTP if (easy->easy_conn->bits.tunnel_connecting) @@ -934,8 +950,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, if(CURLE_OK != easy->result) { /* failure detected */ - Curl_disconnect(easy->easy_conn); /* disconnect properly */ - easy->easy_conn = NULL; /* no more connection */ + disconnect_conn = TRUE; break; } } @@ -964,8 +979,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, if(CURLE_OK != easy->result) { /* failure detected */ - Curl_disconnect(easy->easy_conn); /* close the connection */ - easy->easy_conn = NULL; /* no more connection */ + /* Just break, the cleaning up is handled all in one place */ + disconnect_conn = TRUE; break; } @@ -998,8 +1013,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* failure detected */ Curl_posttransfer(easy->easy_handle); Curl_done(&easy->easy_conn, easy->result, FALSE); - Curl_disconnect(easy->easy_conn); /* close the connection */ - easy->easy_conn = NULL; /* no more connection */ + disconnect_conn = TRUE; } break; @@ -1065,8 +1079,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* failure detected */ Curl_posttransfer(easy->easy_handle); Curl_done(&easy->easy_conn, easy->result, FALSE); - Curl_disconnect(easy->easy_conn); /* close the connection */ - easy->easy_conn = NULL; /* no more connection */ + disconnect_conn = TRUE; } } break; @@ -1098,8 +1111,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* failure detected */ Curl_posttransfer(easy->easy_handle); Curl_done(&easy->easy_conn, easy->result, FALSE); - Curl_disconnect(easy->easy_conn); /* close the connection */ - easy->easy_conn = NULL; /* no more connection */ + disconnect_conn = TRUE; } break; @@ -1208,9 +1220,11 @@ 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 becasue we can't + * 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); if(CURL_SOCKET_BAD != easy->easy_conn->sock[SECONDARYSOCKET]) { /* if we failed anywhere, we must clean up the secondary socket if @@ -1292,10 +1306,17 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* This node should be delinked from the list now and we should post an information message that we are complete. */ + + /* Important: reset the conn pointer so that we don't point to memory + that could be freed anytime */ + easy->easy_conn = NULL; break; case CURLM_STATE_CANCELLED: /* Cancelled transfer, wait to be cleaned up */ + + /* Reset the conn pointer so we don't leave it dangling */ + easy->easy_conn = NULL; break; default: @@ -1308,6 +1329,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, * If an error was returned, and we aren't in completed state now, * then we go to completed and consider this transfer aborted. */ + + /* NOTE: no attempt to disconnect connections must be made + in the case blocks above - cleanup happens only here */ + easy->easy_handle->state.is_in_pipeline = FALSE; easy->easy_handle->state.pipe_broke = FALSE; @@ -1315,7 +1340,21 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* if this has a connection, unsubscribe from the pipelines */ easy->easy_conn->writechannel_inuse = FALSE; easy->easy_conn->readchannel_inuse = FALSE; + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->send_pipe); + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->recv_pipe); } + + if (disconnect_conn) { + Curl_disconnect(easy->easy_conn); /* disconnect properly */ + + /* 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 + failure is detected */ + easy->easy_conn = NULL; + } + multistate(easy, CURLM_STATE_COMPLETED); } } diff --git a/lib/multiif.h b/lib/multiif.h index 800aa0f5d..2a8b06c27 100644 --- a/lib/multiif.h +++ b/lib/multiif.h @@ -31,6 +31,7 @@ void Curl_expire(struct SessionHandle *data, long milli); void Curl_multi_rmeasy(void *multi, CURL *data); bool Curl_multi_canPipeline(struct Curl_multi* multi); +void Curl_multi_handlePipeBreak(struct SessionHandle *data); /* the write bits start at bit 16 for the *getsock() bitmap */ #define GETSOCK_WRITEBITSTART 16 diff --git a/lib/transfer.c b/lib/transfer.c index f4b0380d3..b70f3b509 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -366,6 +366,7 @@ CURLcode Curl_readwrite(struct connectdata *conn, /* receive data from the network! */ readrc = Curl_read(conn, conn->sockfd, k->buf, bytestoread, &nread); + DEBUGF(infof(data, "Read %ld bytes from stream (readrc = %d)\n", nread, readrc)); /* subzero, this would've blocked */ if(0 > readrc) break; /* get out of loop */ @@ -394,7 +395,7 @@ CURLcode Curl_readwrite(struct connectdata *conn, else if (0 >= nread) { /* if we receive 0 or less here, the server closed the connection and we bail out from this! */ - + DEBUGF(infof(data, "nread <= 0, server closed connection, bailing\n")); k->keepon &= ~KEEP_READ; break; } @@ -608,10 +609,10 @@ CURLcode Curl_readwrite(struct connectdata *conn, return result; data->info.header_size += (long)headerlen; - conn->headerbytecount += (long)headerlen; + data->reqdata.keep.headerbytecount += (long)headerlen; - conn->deductheadercount = - (100 == k->httpcode)?conn->headerbytecount:0; + data->reqdata.keep.deductheadercount = + (100 == k->httpcode)?data->reqdata.keep.headerbytecount:0; if (data->reqdata.resume_from && (data->set.httpreq==HTTPREQ_GET) && @@ -1106,7 +1107,7 @@ CURLcode Curl_readwrite(struct connectdata *conn, return result; data->info.header_size += (long)k->hbuflen; - conn->headerbytecount += (long)k->hbuflen; + data->reqdata.keep.headerbytecount += (long)k->hbuflen; /* reset hbufp pointer && hbuflen */ k->hbufp = data->state.headerbuff; @@ -2341,7 +2342,8 @@ bool Curl_retry_request(struct connectdata *conn, bool retry = FALSE; struct SessionHandle *data = conn->data; - if((data->reqdata.keep.bytecount+conn->headerbytecount == 0) && + if((data->reqdata.keep.bytecount + + data->reqdata.keep.headerbytecount == 0) && conn->bits.reuse && !conn->bits.no_body) { /* We got no data, we attempted to re-use a connection and yet we want a diff --git a/lib/url.c b/lib/url.c index 3b6e56642..ddb214d20 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1970,8 +1970,7 @@ static void Curl_printPipeline(struct curl_llist *pipe) curr = pipe->head; while (curr) { struct SessionHandle *data = (struct SessionHandle *) curr->ptr; - infof(data, "Handle in pipeline: %s\n", - data->reqdata.path); + infof(data, "Handle in pipeline: %s\n", data->reqdata.path); curr = curr->next; } } @@ -2010,12 +2009,12 @@ static void signalPipeClose(struct curl_llist *pipe) #ifdef CURLDEBUG /* debug-only code */ if(data->magic != CURLEASY_MAGIC_NUMBER) { /* MAJOR BADNESS */ - fprintf(stderr, "signalPipeClose() found BAAD easy handle\n"); + infof(data, "signalPipeClose() found BAAD easy handle\n"); } - else #endif data->state.pipe_broke = TRUE; + Curl_multi_handlePipeBreak(data); Curl_llist_remove(pipe, curr, NULL); curr = next; } @@ -2060,8 +2059,8 @@ ConnectionExists(struct SessionHandle *data, from the multi */ } - DEBUGF(infof(data, "Examining connection #%ld for reuse, " - "(pipeLen = %ld)\n", check->connectindex, pipeLen)); + DEBUGF(infof(data, "Examining connection #%ld for reuse" + " (pipeLen = %ld)\n", check->connectindex, pipeLen)); if(pipeLen > 0 && !canPipeline) { /* can only happen within multi handles, and means that another easy @@ -2074,12 +2073,26 @@ ConnectionExists(struct SessionHandle *data, yet and until then we don't re-use this connection */ if (!check->ip_addr_str) { infof(data, - "Connection #%ld has not finished name resolve, can't reuse\n", + "Connection #%ld hasn't finished name resolve, can't reuse\n", check->connectindex); continue; } #endif + if ((check->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) || check->bits.close) { + /* Don't pick a connection that hasn't connected yet or that is going to + get closed. */ + infof(data, "Connection #%ld isn't open enough, can't reuse\n", + check->connectindex); +#ifdef CURLDEBUG + if (check->recv_pipe->size > 0) { + infof(data, "BAD! Unconnected #%ld has a non-empty recv pipeline!\n", + check->connectindex); + } +#endif + continue; + } + if (pipeLen >= MAX_PIPELINE_LENGTH) { infof(data, "Connection #%ld has its pipeline full, can't reuse\n", check->connectindex); @@ -2100,13 +2113,6 @@ ConnectionExists(struct SessionHandle *data, } } - if (check->bits.close) { - /* Don't pick a connection that is going to be closed. */ - infof(data, "Connection #%ld has been marked for close, can't reuse\n", - check->connectindex); - continue; - } - if((needle->protocol&PROT_SSL) != (check->protocol&PROT_SSL)) /* don't do mixed SSL and non-SSL connections */ continue; @@ -2178,15 +2184,11 @@ ConnectionExists(struct SessionHandle *data, check->inuse = TRUE; /* mark this as being in use so that no other handle in a multi stack may nick it */ - if (canPipeline) { /* Mark the connection as being in a pipeline */ check->is_in_pipeline = TRUE; } - check->connectindex = i; /* Set this appropriately since it might have - been set to -1 when the easy was removed - from the multi */ *usethis = check; return TRUE; /* yes, we found one to use! */ } @@ -4069,7 +4071,7 @@ static CURLcode SetupConnection(struct connectdata *conn, return CURLE_OUT_OF_MEMORY; } - conn->headerbytecount = 0; + data->reqdata.keep.headerbytecount = 0; #ifdef CURL_DO_LINEEND_CONV data->state.crlf_conversions = 0; /* reset CRLF conversion counter */ diff --git a/lib/urldata.h b/lib/urldata.h index 879f36902..419915569 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -555,6 +555,13 @@ struct Curl_transfer_keeper { curl_off_t bytecount; /* total number of bytes read */ curl_off_t writebytecount; /* number of bytes written */ + long headerbytecount; /* only count received headers */ + long deductheadercount; /* this amount of bytes doesn't count when we check + if anything has been transfered at the end of + a connection. We use this counter to make only + a 100 reply (without a following second response + code) result in a CURLE_GOT_NOTHING error code */ + struct timeval start; /* transfer started at this time */ struct timeval now; /* current time */ bool header; /* incoming data has HTTP header */ @@ -756,13 +763,6 @@ struct connectdata { unsigned short remote_port; /* what remote port to connect to, not the proxy port! */ - long headerbytecount; /* only count received headers */ - long deductheadercount; /* this amount of bytes doesn't count when we check - if anything has been transfered at the end of - a connection. We use this counter to make only - a 100 reply (without a following second response - code) result in a CURLE_GOT_NOTHING error code */ - char *user; /* user name string, allocated */ char *passwd; /* password string, allocated */