Commit 71920d61 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

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.
parent 5de75eee

lib/multi.c

+7 −10
Original line number Diff line number Diff line
@@ -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 */

lib/url.c

+73 −9
Original line number Diff line number Diff line
@@ -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,6 +1917,13 @@ static void signalPipeClose(struct curl_llist *pipe) 
    struct curl_llist_element *next = curr->next;

    struct SessionHandle *data = (struct SessionHandle *) curr->ptr;



#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);
@@ -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;