Unverified Commit acefdd0c authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

multi: always do the COMPLETED procedure/state 

It was previously erroneously skipped in some situations.

libtest/libntlmconnect.c wrongly depended on wrong behavior (that it
would get a zero timeout) when no handles are "running" in a multi
handle. That behavior is no longer present with this fix. Now libcurl
will always return a -1 timeout when all handles are completed.

Closes #2733
parent 151d3c56

lib/multi.c

+30 −23
Original line number Original line Diff line number Diff line
@@ -107,6 +107,16 @@ static const char * const statename[]={ 
/* function pointer called once when switching TO a state */

/* function pointer called once when switching TO a state */

typedef void (*init_multistate_func)(struct Curl_easy *data);

typedef void (*init_multistate_func)(struct Curl_easy *data);





static void Curl_init_completed(struct Curl_easy *data)

{

  /* this is a completed transfer */



  /* Important: reset the conn pointer so that we don't point to memory

     that could be freed anytime */

  data->easy_conn = NULL;

  Curl_expire_clear(data); /* stop all timers */

}



/* always use this function to change state, to make debugging easier */

/* always use this function to change state, to make debugging easier */

static void mstate(struct Curl_easy *data, CURLMstate state

static void mstate(struct Curl_easy *data, CURLMstate state

#ifdef DEBUGBUILD

#ifdef DEBUGBUILD
@@ -116,17 +126,25 @@ static void mstate(struct Curl_easy *data, CURLMstate state 
{

{

  CURLMstate oldstate = data->mstate;

  CURLMstate oldstate = data->mstate;

  static const init_multistate_func finit[CURLM_STATE_LAST] = {

  static const init_multistate_func finit[CURLM_STATE_LAST] = {

    NULL,

    NULL,              /* INIT */

    NULL,

    NULL,              /* CONNECT_PEND */

    Curl_init_CONNECT, /* CONNECT */

    Curl_init_CONNECT, /* CONNECT */

    NULL,

    NULL,              /* WAITRESOLVE */

    NULL,

    NULL,              /* WAITCONNECT */

    NULL,

    NULL,              /* WAITPROXYCONNECT */

    NULL,

    NULL,              /* SENDPROTOCONNECT */

    NULL,

    NULL,              /* PROTOCONNECT */

    NULL,

    NULL,              /* WAITDO */

    Curl_connect_free /* DO */

    Curl_connect_free, /* DO */

    /* the rest is NULL too */

    NULL,              /* DOING */

    NULL,              /* DO_MORE */

    NULL,              /* DO_DONE */

    NULL,              /* WAITPERFORM */

    NULL,              /* PERFORM */

    NULL,              /* TOOFAST */

    NULL,              /* DONE */

    Curl_init_completed, /* COMPLETED */

    NULL               /* MSGSENT */

  };

  };





#if defined(DEBUGBUILD) && defined(CURL_DISABLE_VERBOSE_STRINGS)

#if defined(DEBUGBUILD) && defined(CURL_DISABLE_VERBOSE_STRINGS)
@@ -2062,16 +2080,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
      break;

      break;





    case CURLM_STATE_COMPLETED:

    case CURLM_STATE_COMPLETED:

      /* this is a completed transfer, it is likely to still be connected */



      /* 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 */

      data->easy_conn = NULL;



      Curl_expire_clear(data); /* stop all timers */

      break;

      break;





    case CURLM_STATE_MSGSENT:

    case CURLM_STATE_MSGSENT:
@@ -2123,6 +2131,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
        }

        }





        multistate(data, CURLM_STATE_COMPLETED);

        multistate(data, CURLM_STATE_COMPLETED);

        rc = CURLM_CALL_MULTI_PERFORM;

      }

      }

      /* if there's still a connection to use, call the progress function */

      /* if there's still a connection to use, call the progress function */

      else if(data->easy_conn && Curl_pgrsUpdate(data->easy_conn)) {

      else if(data->easy_conn && Curl_pgrsUpdate(data->easy_conn)) {
@@ -2147,14 +2156,12 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
      msg->extmsg.data.result = result;

      msg->extmsg.data.result = result;





      rc = multi_addmsg(multi, msg);

      rc = multi_addmsg(multi, msg);



      DEBUGASSERT(!data->easy_conn);

      multistate(data, CURLM_STATE_MSGSENT);

      multistate(data, CURLM_STATE_MSGSENT);

    }

    }

  } while((rc == CURLM_CALL_MULTI_PERFORM) || multi_ischanged(multi, FALSE));

  } while((rc == CURLM_CALL_MULTI_PERFORM) || multi_ischanged(multi, FALSE));





  data->result = result;

  data->result = result;





  return rc;

  return rc;

}

}

tests/libtest/libntlmconnect.c

+10 −5
Original line number Original line Diff line number Diff line
@@ -5,7 +5,7 @@ 
 *                            | (__| |_| |  _ <| |___

 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *                             \___|\___/|_| \_\_____|

 *

 *

 * Copyright (C) 2012 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.

 * Copyright (C) 2012 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.

 *

 *

 * This software is licensed as described in the file COPYING, which

 * This software is licensed as described in the file COPYING, which

 * you should have received as part of this distribution. The terms

 * you should have received as part of this distribution. The terms
@@ -158,6 +158,9 @@ int test(char *url) 




    multi_perform(multi, &running);

    multi_perform(multi, &running);





    fprintf(stderr, "%s:%d running %ld state %d\n",

            __FILE__, __LINE__, running, state);



    abort_on_test_timeout();

    abort_on_test_timeout();





    if(!running && state == NoMoreHandles)

    if(!running && state == NoMoreHandles)
@@ -179,14 +182,16 @@ int test(char *url) 
      }

      }

      state = num_handles < MAX_EASY_HANDLES ? ReadyForNewHandle

      state = num_handles < MAX_EASY_HANDLES ? ReadyForNewHandle

                                             : NoMoreHandles;

                                             : NoMoreHandles;

      fprintf(stderr, "%s:%d new state %d\n",

              __FILE__, __LINE__, state);

    }

    }





    multi_timeout(multi, &timeout);

    multi_timeout(multi, &timeout);





    /* At this point, timeout is guaranteed to be greater or equal than -1. */

    /* At this point, timeout is guaranteed to be greater or equal than -1. */





    fprintf(stderr, "%s:%d num_handles %d timeout %ld\n",

    fprintf(stderr, "%s:%d num_handles %d timeout %ld running %d\n",

            __FILE__, __LINE__, num_handles, timeout);

            __FILE__, __LINE__, num_handles, timeout, running);





    if(timeout != -1L) {

    if(timeout != -1L) {

      int itimeout = (timeout > (long)INT_MAX) ? INT_MAX : (int)timeout;

      int itimeout = (timeout > (long)INT_MAX) ? INT_MAX : (int)timeout;
@@ -194,8 +199,8 @@ int test(char *url) 
      interval.tv_usec = (itimeout%1000)*1000;

      interval.tv_usec = (itimeout%1000)*1000;

    }

    }

    else {

    else {

      interval.tv_sec = TEST_HANG_TIMEOUT/1000 + 1;

      interval.tv_sec = 0;

      interval.tv_usec = 0;

      interval.tv_usec = 5000;





      /* if there's no timeout and we get here on the last handle, we may

      /* if there's no timeout and we get here on the last handle, we may

         already have read the last part of the stream so waiting makes no

         already have read the last part of the stream so waiting makes no