From 71920d61e6c0f38f91a7ba471505de031d568ff7 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 20 Sep 2006 12:03:50 +0000 Subject: Michael Wallner's test program again help me track down a problem. This time it basically was that we didn't remove the current connection from the pipe list when following a redirect. Also in this commit: several cases of additional debug code for debug builds helping to check and track down some signs of run-time trouble. --- lib/multi.c | 17 ++++++------- lib/url.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/lib/multi.c b/lib/multi.c index 2f7f32870..2da17a3f0 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -424,12 +424,12 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle, /* increase the node-counter */ multi->num_easy++; - if((multi->num_easy+5) > multi->connc->num) { - /* we want the connection cache to have room for all easy transfers, and - some more so we have a margin of 5 for now, but we add the new amount - plus 10 to not have to do it for every new handle added */ + if((multi->num_easy * 4) > multi->connc->num) { + /* We want the connection cache to have plenty room. Before we supported + the shared cache every single easy handle had 5 entries in their cache + by default. */ CURLcode res = Curl_ch_connc(easy_handle, multi->connc, - multi->num_easy + 10); + multi->connc->num*4); if(res) /* TODO: we need to do some cleaning up here! */ return res; @@ -1111,13 +1111,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, /* call this even if the readwrite function returned error */ Curl_posttransfer(easy->easy_handle); - if (retry) { - Curl_removeHandleFromPipeline(easy->easy_handle, - easy->easy_conn->recv_pipe); - } - /* When we follow redirects, must to go back to the CONNECT state */ if(easy->easy_handle->reqdata.newurl || retry) { + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->recv_pipe); if(!retry) { /* if the URL is a follow-location and not just a retried request then figure out the URL here */ diff --git a/lib/url.c b/lib/url.c index 3f59299be..71a827e7a 100644 --- a/lib/url.c +++ b/lib/url.c @@ -216,6 +216,49 @@ CURLcode Curl_close(struct SessionHandle *data) { struct Curl_multi *m = data->multi; +#ifdef CURLDEBUG + /* only for debugging, scan through all connections and see if there's a + pipe reference still identifying this handle */ + + if(data->state.is_in_pipeline) + fprintf(stderr, "CLOSED when in pipeline!\n"); + + if(data->state.connc && data->state.connc->type == CONNCACHE_MULTI) { + struct conncache *c = data->state.connc; + int i; + struct curl_llist *pipe; + struct curl_llist_element *curr; + struct connectdata *connptr; + + for(i=0; i< c->num; i++) { + connptr = c->connects[i]; + if(!connptr) + continue; + + pipe = connptr->send_pipe; + if(pipe) { + for (curr = pipe->head; curr; curr=curr->next) { + if(data == (struct SessionHandle *) curr->ptr) { + fprintf(stderr, + "MAJOR problem we %p are still in send pipe for %p done %d\n", + data, connptr, connptr->bits.done); + } + } + } + pipe = connptr->recv_pipe; + if(pipe) { + for (curr = pipe->head; curr; curr=curr->next) { + if(data == (struct SessionHandle *) curr->ptr) { + fprintf(stderr, + "MAJOR problem we %p are still in recv pipe for %p done %d\n", + data, connptr, connptr->bits.done); + } + } + } + } + } +#endif + if(m) /* This handle is still part of a multi handle, take care of this first and detach this handle from there. */ @@ -1707,6 +1750,11 @@ CURLcode Curl_disconnect(struct connectdata *conn) return CURLE_OK; /* this is closed and fine already */ data = conn->data; + if(!data) { + DEBUGF(infof(data, "DISCONNECT without easy handle, ignoring\n")); + return CURLE_OK; + } + #if defined(CURLDEBUG) && defined(AGGRESIVE_TEST) /* scan for DNS cache entries still marked as in use */ Curl_hash_apply(data->hostcache, @@ -1805,12 +1853,17 @@ static bool IsPipeliningPossible(struct SessionHandle *handle) return FALSE; } -void Curl_addHandleToPipeline(struct SessionHandle *handle, +void Curl_addHandleToPipeline(struct SessionHandle *data, struct curl_llist *pipe) { - Curl_llist_insert_next(pipe, - pipe->tail, - handle); +#ifdef CURLDEBUG + if(!IsPipeliningPossible(data)) { + /* when not pipelined, there MUST be no handle in the list already */ + if(pipe->head) + infof(data, "PIPE when no PIPE supposed!\n"); + } +#endif + Curl_llist_insert_next(pipe, pipe->tail, data); } @@ -1864,7 +1917,14 @@ static void signalPipeClose(struct curl_llist *pipe) struct curl_llist_element *next = curr->next; struct SessionHandle *data = (struct SessionHandle *) curr->ptr; - data->state.pipe_broke = TRUE; +#ifdef CURLDEBUG /* debug-only code */ + if(data->magic != CURLEASY_MAGIC_NUMBER) { + /* MAJOR BADNESS */ + fprintf(stderr, "signalPipeClose() found BAAD easy handle\n"); + } + else +#endif + data->state.pipe_broke = TRUE; Curl_llist_remove(pipe, curr, NULL); curr = next; @@ -2062,7 +2122,7 @@ ConnectionDone(struct connectdata *conn) if (conn->send_pipe == 0 && conn->recv_pipe == 0) - conn->is_in_pipeline = FALSE; + conn->is_in_pipeline = FALSE; } /* @@ -2086,7 +2146,8 @@ ConnectionStore(struct SessionHandle *data, /* there was no room available, kill one */ i = ConnectionKillOne(data); if(-1 != i) - infof(data, "Connection (#%d) was killed to make room\n", i); + infof(data, "Connection (#%d) was killed to make room (holds %d)\n", + i, data->state.connc->num); else infof(data, "This connection did not fit in the connection cache\n"); } @@ -3541,7 +3602,10 @@ static CURLcode CreateConnection(struct SessionHandle *data, /* Setup a "faked" transfer that'll do nothing */ if(CURLE_OK == result) { + conn->data = data; conn->bits.tcpconnect = TRUE; /* we are "connected */ + ConnectionStore(data, conn); + result = Curl_setup_transfer(conn, -1, -1, FALSE, NULL, /* no download */ -1, NULL); /* no upload */ } @@ -4438,6 +4502,8 @@ CURLcode Curl_done(struct connectdata **connp, cancelled before we proceed */ ares_cancel(data->state.areschannel); + ConnectionDone(conn); /* the connection is no longer in use */ + /* if data->set.reuse_forbid is TRUE, it means the libcurl client has forced us to close this no matter what we think. @@ -4463,8 +4529,6 @@ CURLcode Curl_done(struct connectdata **connp, infof(data, "Connection #%ld to host %s left intact\n", conn->connectindex, conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname); - - ConnectionDone(conn); /* the connection is no longer in use */ } return result; -- cgit v1.2.3