From e1edd41e1b9357cec2d42762e8ae67e356693941 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 23 Oct 2006 20:34:56 +0000 Subject: Ravi Pratap provided a major update with pipelining fixes. We also no longer re-use connections (for pipelining) before the name resolving is done. --- CHANGES | 4 ++++ lib/multi.c | 3 ++- lib/sendf.c | 69 ++++++++++++++++++++++++++++++++++------------------------ lib/transfer.c | 26 +++++++++++++--------- lib/url.c | 58 +++++++++++++++++++++++++++++++++--------------- lib/urldata.h | 4 ++-- 6 files changed, 104 insertions(+), 60 deletions(-) diff --git a/CHANGES b/CHANGES index 75affc1d2..6199e9c8e 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,10 @@ Changelog +Daniel (23 October 2006) +- Ravi Pratap provided a major update with pipelining fixes. We also no longer + re-use connections (for pipelining) before the name resolving is done. + Daniel (21 October 2006) - Nir Soffer made the tests/libtest/Makefile.am use a proper variable for all the single test applications' link and dependences, so that you easier can diff --git a/lib/multi.c b/lib/multi.c index 6daabd00b..a7cc25562 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -773,7 +773,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, return CURLM_BAD_EASY_HANDLE; if (easy->easy_handle->state.pipe_broke) { - infof(easy->easy_handle, "Pipe broke: handle 0x%x\n", easy); + 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); diff --git a/lib/sendf.c b/lib/sendf.c index 934fad1bb..01a23e445 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -438,14 +438,25 @@ CURLcode Curl_client_write(struct connectdata *conn, return CURLE_OK; } +#define MIN(a,b) (a < b ? a : b) + void Curl_read_rewind(struct connectdata *conn, size_t extraBytesRead) { + char buf[512 + 1]; + size_t bytesToShow; + conn->read_pos -= extraBytesRead; conn->bits.stream_was_rewound = TRUE; -} -#define MIN(a,b) (a < b ? a : b) + bytesToShow = MIN(conn->buf_len - conn->read_pos, sizeof(buf)-1); + memcpy(buf, conn->master_buffer + conn->read_pos, bytesToShow); + buf[bytesToShow] = '\0'; + + DEBUGF(infof(conn->data, + "Buffer after stream rewind (read_pos = %d): [%s]", + conn->read_pos, buf)); +} /* * Internal read-from-socket function. This is meant to deal with plain @@ -457,12 +468,12 @@ void Curl_read_rewind(struct connectdata *conn, int Curl_read(struct connectdata *conn, /* connection data */ curl_socket_t sockfd, /* read from this socket */ char *buf, /* store read data here */ - size_t buffersize, /* max amount to read */ + size_t sizerequested, /* max amount to read */ ssize_t *n) /* amount bytes read */ { ssize_t nread; - size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, buffersize); - size_t bytesremaining = buffersize - bytestocopy; + size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, sizerequested); + size_t bytesfromsocket = 0; /* Set 'num' to 0 or 1, depending on which socket that has been sent here. If it is the second socket, we set num to 1. Otherwise to 0. This lets @@ -471,34 +482,34 @@ int Curl_read(struct connectdata *conn, /* connection data */ *n=0; /* reset amount to zero */ - bytesremaining = MIN(bytesremaining, sizeof(conn->master_buffer)); - - /* Copy from our master buffer first */ - memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy); - conn->read_pos += bytestocopy; + /* Copy from our master buffer first if we have some unread data there*/ + if (bytestocopy > 0) { + memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy); + conn->read_pos += bytestocopy; + conn->bits.stream_was_rewound = FALSE; - conn->bits.stream_was_rewound = FALSE; - - *n = (ssize_t)bytestocopy; - - if (bytesremaining == 0) { - return CURLE_OK; + *n = (ssize_t)bytestocopy; + return CURLE_OK; } + /* If we come here, it means that there is no data to read from the buffer, + * so we read from the socket */ + bytesfromsocket = MIN(sizerequested, sizeof(conn->master_buffer)); + if(conn->ssl[num].use) { - nread = Curl_ssl_recv(conn, num, conn->master_buffer, bytesremaining); + nread = Curl_ssl_recv(conn, num, conn->master_buffer, bytesfromsocket); - if(nread == -1 && bytestocopy == 0) { + if(nread == -1) return -1; /* -1 from Curl_ssl_recv() means EWOULDBLOCK */ - } - - } else { + } + else { if(conn->sec_complete) - nread = Curl_sec_read(conn, sockfd, conn->master_buffer, bytesremaining); + nread = Curl_sec_read(conn, sockfd, conn->master_buffer, + bytesfromsocket); else - nread = sread(sockfd, conn->master_buffer, bytesremaining); + nread = sread(sockfd, conn->master_buffer, bytesfromsocket); - if(-1 == nread && bytestocopy == 0) { + if(-1 == nread) { int err = Curl_sockerrno(); #ifdef USE_WINSOCK if(WSAEWOULDBLOCK == err) @@ -509,12 +520,12 @@ int Curl_read(struct connectdata *conn, /* connection data */ } } - if (nread > 0) { - memcpy(buf, conn->master_buffer, nread); + if (nread >= 0) { + memcpy(buf, conn->master_buffer, nread); - conn->buf_len = nread; - conn->read_pos = nread; - *n += nread; + conn->buf_len = nread; + conn->read_pos = nread; + *n = nread; } return CURLE_OK; diff --git a/lib/transfer.c b/lib/transfer.c index e136020b0..300b8e9f6 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -743,8 +743,10 @@ CURLcode Curl_readwrite(struct connectdata *conn, failf(data, "Maximum file size exceeded"); return CURLE_FILESIZE_EXCEEDED; } - if(contentlength >= 0) + if(contentlength >= 0) { k->size = contentlength; + k->maxdownload = k->size; + } else { /* Negative Content-Length is really odd, and we know it happens for example when older Apache servers send large @@ -1091,11 +1093,11 @@ CURLcode Curl_readwrite(struct connectdata *conn, Curl_debug(data, CURLINFO_DATA_IN, data->state.headerbuff, (size_t)k->hbuflen, conn); if(k->badheader == HEADER_PARTHEADER) - Curl_debug(data, CURLINFO_DATA_IN, + Curl_debug(data, CURLINFO_DATA_IN, k->str, (size_t)nread, conn); } else - Curl_debug(data, CURLINFO_DATA_IN, + Curl_debug(data, CURLINFO_DATA_IN, k->str, (size_t)nread, conn); } @@ -1133,13 +1135,17 @@ CURLcode Curl_readwrite(struct connectdata *conn, if((-1 != k->maxdownload) && (k->bytecount + nread >= k->maxdownload)) { - size_t excess = (size_t)(k->bytecount + - (curl_off_t)nread - k->maxdownload); - - if (excess > 0) { - infof(data, "Rewinding stream by : %d bytes\n", excess); - Curl_read_rewind(conn, excess); - conn->bits.stream_was_rewound = TRUE; + curl_off_t excess = k->bytecount + + ((curl_off_t) nread) - k->maxdownload; + if (excess > 0 && !k->ignorebody) { + infof(data, + "Rewinding stream by : %" FORMAT_OFF_T + " bytes on url %s (size = %" FORMAT_OFF_T + ", maxdownload = %" FORMAT_OFF_T + ", bytecount = %" FORMAT_OFF_T ", nread = %d)\n", + excess, conn->data->reqdata.path, + k->size, k->maxdownload, k->bytecount, nread); + Curl_read_rewind(conn, excess); } nread = (ssize_t) (k->maxdownload - k->bytecount); diff --git a/lib/url.c b/lib/url.c index 65453155c..4ca5d56a7 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1854,6 +1854,7 @@ int Curl_removeHandleFromPipeline(struct SessionHandle *handle, } curr = curr->next; } + return 0; } @@ -1899,8 +1900,8 @@ static void signalPipeClose(struct curl_llist *pipe) } else #endif - data->state.pipe_broke = TRUE; + data->state.pipe_broke = TRUE; Curl_llist_remove(pipe, curr, NULL); curr = next; } @@ -1936,12 +1937,19 @@ ConnectionExists(struct SessionHandle *data, /* NULL pointer means not filled-in entry */ continue; +#ifdef USE_ARES + /* ip_addr_str is NULL only if the resolving of the name hasn't completed + yet and until then we don't re-use this connection */ + if (!check->ip_addr_str) + continue; +#endif + if(check->inuse && !canPipeline) /* can only happen within multi handles, and means that another easy handle is using this connection */ continue; - if (check->send_pipe->size >= MAX_PIPELINE_LENGTH || + if (check->send_pipe->size + check->recv_pipe->size >= MAX_PIPELINE_LENGTH) continue; @@ -1994,14 +2002,28 @@ ConnectionExists(struct SessionHandle *data, } if(match) { + bool dead = SocketIsDead(check->sock[FIRSTSOCKET]); if(dead) { - /* - */ - check->data = data; - infof(data, "Connection %d seems to be dead!\n", i); - Curl_disconnect(check); /* disconnect resources */ - data->state.connc->connects[i]=NULL; /* nothing here */ + if (!check->is_in_pipeline) { + check->data = data; + infof(data, "Connection %d seems to be dead!\n", i); + + Curl_disconnect(check); /* disconnect resources */ + data->state.connc->connects[i]=NULL; /* nothing here */ + } + else { + /* In the pipelining case, freeing the connection right away + * doesn't work. Instead, we mark the connection for close + * so that the handles will detect this automatically when + * they try to do something and deal with it there. + * Prematurely freeing this connection prevents handles that + * have already finished with their transfers to shut down + * cleanly. + */ + infof(data, "Connection %d seems dead - marking for close\n", i); + check->bits.close = TRUE; + } /* There's no need to continue searching, because we only store one connection for each unique set of identifiers */ @@ -2012,10 +2034,13 @@ ConnectionExists(struct SessionHandle *data, handle in a multi stack may nick it */ if (canPipeline) { - /* Mark the connection as being in a pipeline */ - check->is_in_pipeline = TRUE; + /* 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! */ } @@ -2690,6 +2715,9 @@ static CURLcode CreateConnection(struct SessionHandle *data, conn->readchannel_inuse = FALSE; conn->writechannel_inuse = FALSE; + conn->read_pos = 0; + conn->buf_len = 0; + /* Initialize the pipeline lists */ conn->send_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); conn->recv_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); @@ -3622,12 +3650,6 @@ static CURLcode CreateConnection(struct SessionHandle *data, infof(data, "Re-using existing connection! (#%ld) with host %s\n", conn->connectindex, conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname); -#ifdef CURLRES_ASYNCH - if(!conn->ip_addr_str) { - infof(data, "... but it is not resolved yet!\n"); - *async = TRUE; - } -#endif } else { /* @@ -3990,10 +4012,10 @@ CURLcode Curl_done(struct connectdata **connp, if(Curl_removeHandleFromPipeline(data, conn->recv_pipe) && conn->readchannel_inuse) - conn->readchannel_inuse--; + conn->readchannel_inuse = FALSE; if(Curl_removeHandleFromPipeline(data, conn->send_pipe) && conn->writechannel_inuse) - conn->writechannel_inuse--; + conn->writechannel_inuse = FALSE; /* cleanups done even if the connection is re-used */ if(data->reqdata.rangestringalloc) { diff --git a/lib/urldata.h b/lib/urldata.h index c1b52dbf7..c93b46dff 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -812,8 +812,8 @@ struct connectdata { their responses on this pipeline */ char master_buffer[BUFSIZE]; /* The master buffer for this connection. */ - size_t read_pos; - size_t buf_len; + size_t read_pos; /* Current read position in the master buffer */ + size_t buf_len; /* Length of the buffer?? */ /*************** Request - specific items ************/ -- cgit v1.2.3