Unverified Commit 094b5f35 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

multi: make sure 'data' can present in several sockhash entries 

Since more than one socket can be used by each transfer at a given time,
each sockhash entry how has its own hash table with transfers using that
socket.

In addition, the sockhash entry can now be marked 'blocked = TRUE'"
which then makes the delete function just set 'removed = TRUE' instead
of removing it "for real", as a way to not rip out the carpet under the
feet of a parent function that iterates over the transfers of that same
sockhash entry.

Reported-by: Tom van der Woerdt
Fixes #3961
Fixes #3986
Fixes #3995
Fixes #4004
Closes #3997
parent fb2839db

lib/multi.c

+94 −65
Original line number Diff line number Diff line
@@ -187,13 +187,16 @@ static void mstate(struct Curl_easy *data, CURLMstate state 
 */



struct Curl_sh_entry {

  struct curl_llist list; /* list of easy handles using this socket */

  struct curl_hash transfers; /* hash of transfers using this socket */

  unsigned int action;  /* what combined action READ/WRITE this socket waits

                           for */

  void *socketp; /* settable by users with curl_multi_assign() */

  unsigned int users; /* number of transfers using this */

  unsigned int readers; /* this many transfers want to read */

  unsigned int writers; /* this many transfers want to write */

  unsigned int blocked:1; /* if TRUE, blocked from being removed */

  unsigned int removed:1; /* if TRUE, this entry is "removed" but prevented

                             from it by "blocked" being set! */

};

/* bits for 'action' having no bits means this socket is not expecting any

   action */
@@ -202,31 +205,67 @@ struct Curl_sh_entry { 


/* look up a given socket in the socket hash, skip invalid sockets */

static struct Curl_sh_entry *sh_getentry(struct curl_hash *sh,

                                         curl_socket_t s)

                                         curl_socket_t s,

                                         bool also_hidden)

{

  if(s != CURL_SOCKET_BAD)

  if(s != CURL_SOCKET_BAD) {

    /* only look for proper sockets */

    return Curl_hash_pick(sh, (char *)&s, sizeof(curl_socket_t));

    struct Curl_sh_entry *entry =

      Curl_hash_pick(sh, (char *)&s, sizeof(curl_socket_t));

    if(entry && entry->removed && !also_hidden)

      return NULL;

    return entry;

  }

  return NULL;

}



#define TRHASH_SIZE 13

static size_t trhash(void *key, size_t key_length, size_t slots_num)

{

  size_t keyval = (size_t)key; /* this is a data pointer */

  (void) key_length;



  return (keyval % slots_num);

}



static size_t trhash_compare(void *k1, size_t k1_len, void *k2, size_t k2_len)

{

  (void)k1_len;

  (void)k2_len;



  return k1 == k2;

}



static void trhash_dtor(void *nada)

{

  (void)nada;

}





/* make sure this socket is present in the hash for this handle */

static struct Curl_sh_entry *sh_addentry(struct curl_hash *sh,

                                         curl_socket_t s)

{

  struct Curl_sh_entry *there = sh_getentry(sh, s);

  struct Curl_sh_entry *there = sh_getentry(sh, s, TRUE);

  struct Curl_sh_entry *check;



  if(there)

  if(there) {

    /* it is present, return fine */

    if(there->removed)

      there->removed = FALSE; /* clear the removed bit */

    return there;

  }



  /* not present, add it */

  check = calloc(1, sizeof(struct Curl_sh_entry));

  if(!check)

    return NULL; /* major failure */



  Curl_llist_init(&check->list, NULL);

  if(Curl_hash_init(&check->transfers, TRHASH_SIZE, trhash,

                    trhash_compare, trhash_dtor)) {

    free(check);

    return NULL;

  }



  /* make/add new hash entry */

  if(!Curl_hash_add(sh, (char *)&s, sizeof(curl_socket_t), check)) {
@@ -242,18 +281,18 @@ static struct Curl_sh_entry *sh_addentry(struct curl_hash *sh, 
static void sh_delentry(struct Curl_sh_entry *entry,

                        struct curl_hash *sh, curl_socket_t s)

{

  struct curl_llist *list = &entry->list;

  struct curl_llist_element *e;

  /* clear the list of transfers first */

  for(e = list->head; e; e = list->head) {

    struct Curl_easy *dta = e->ptr;

    Curl_llist_remove(&entry->list, e, NULL);

    dta->sh_entry = NULL;

  if(entry->blocked) {

    entry->removed = TRUE; /* pretend */

    return;

  }

  else {

    Curl_hash_destroy(&entry->transfers);



    /* We remove the hash entry. This will end up in a call to

       sh_freeentry(). */

    Curl_hash_delete(sh, (char *)&s, sizeof(curl_socket_t));

  }

}



/*

 * free a sockhash entry
@@ -318,17 +357,6 @@ static CURLMcode multi_addmsg(struct Curl_multi *multi, 
  return CURLM_OK;

}



/*

 * multi_freeamsg()

 *

 * Callback used by the llist system when a single list entry is destroyed.

 */

static void multi_freeamsg(void *a, void *b)

{

  (void)a;

  (void)b;

}



struct Curl_multi *Curl_multi_handle(int hashsize, /* socket hash */

                                     int chashsize) /* connection hash */

{
@@ -348,8 +376,8 @@ struct Curl_multi *Curl_multi_handle(int hashsize, /* socket hash */ 
  if(Curl_conncache_init(&multi->conn_cache, chashsize))

    goto error;



  Curl_llist_init(&multi->msglist, multi_freeamsg);

  Curl_llist_init(&multi->pending, multi_freeamsg);

  Curl_llist_init(&multi->msglist, NULL);

  Curl_llist_init(&multi->pending, NULL);



  /* -1 means it not set by user, use the default value */

  multi->maxconnects = -1;
@@ -787,11 +815,6 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi) 
static void detach_connnection(struct Curl_easy *data)

{

  struct connectdata *conn = data->conn;

  if(data->sh_entry) {

    /* still listed as a user of a socket hash entry, remove it */

    Curl_llist_remove(&data->sh_entry->list, &data->sh_queue, NULL);

    data->sh_entry = NULL;

  }

  if(conn)

    Curl_llist_remove(&conn->easyq, &data->conn_queue, NULL);

  data->conn = NULL;
@@ -1264,6 +1287,9 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, 
    bool stream_error = FALSE;

    rc = CURLM_OK;



    DEBUGASSERT((data->mstate <= CURLM_STATE_CONNECT) ||

                (data->mstate >= CURLM_STATE_DONE) ||

                data->conn);

    if(!data->conn &&

       data->mstate > CURLM_STATE_CONNECT &&

       data->mstate < CURLM_STATE_DONE) {
@@ -2240,7 +2266,7 @@ static CURLMcode singlesocket(struct Curl_multi *multi, 
    s = socks[i];



    /* get it from the hash */

    entry = sh_getentry(&multi->sockhash, s);

    entry = sh_getentry(&multi->sockhash, s, FALSE);



    if(curraction & GETSOCK_READSOCK(i))

      action |= CURL_POLL_IN;
@@ -2285,30 +2311,22 @@ static CURLMcode singlesocket(struct Curl_multi *multi, 
      if(action & CURL_POLL_OUT)

        entry->writers++;



      /* add 'data' to the list of handles using this socket! */

      Curl_llist_insert_next(&entry->list, entry->list.tail,

                             data, &data->sh_queue);

      data->sh_entry = entry;

      /* add 'data' to the transfer hash on this socket! */

      if(!Curl_hash_add(&entry->transfers, (char *)data, /* hash key */

                        sizeof(struct Curl_easy *), data))

        return CURLM_OUT_OF_MEMORY;

    }



    comboaction = (entry->writers? CURL_POLL_OUT : 0) |

      (entry->readers ? CURL_POLL_IN : 0);



#if 0

    infof(data, "--- Comboaction: %u readers %u writers\n",

          entry->readers, entry->writers);

#endif

    /* check if it has the same action set */

    if(entry->action == comboaction)

    /* socket existed before and has the same action set as before */

    if(sincebefore && (entry->action == comboaction))

      /* same, continue */

      continue;



    /* we know (entry != NULL) at this point, see the logic above */

    if(multi->socket_cb)

      multi->socket_cb(data,

                       s,

                       comboaction,

                       multi->socket_userp,

      multi->socket_cb(data, s, comboaction, multi->socket_userp,

                       entry->socketp);



    entry->action = comboaction; /* store the current action state */
@@ -2332,7 +2350,7 @@ static CURLMcode singlesocket(struct Curl_multi *multi, 
    if(stillused)

      continue;



    entry = sh_getentry(&multi->sockhash, s);

    entry = sh_getentry(&multi->sockhash, s, FALSE);

    /* if this is NULL here, the socket has been closed and notified so

       already by Curl_multi_closed() */

    if(entry) {
@@ -2350,6 +2368,11 @@ static CURLMcode singlesocket(struct Curl_multi *multi, 
                           entry->socketp);

        sh_delentry(entry, &multi->sockhash, s);

      }

      else {

        /* still users, but remove this handle as a user of this socket */

        Curl_hash_delete(&entry->transfers, (char *)data,

                         sizeof(struct Curl_easy *));

      }

    }

  } /* for loop over numsocks */
@@ -2383,7 +2406,7 @@ void Curl_multi_closed(struct Curl_easy *data, curl_socket_t s) 
    if(multi) {

      /* this is set if this connection is part of a handle that is added to

         a multi handle, and only then this is necessary */

      struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);

      struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s, FALSE);



      if(entry) {

        if(multi->socket_cb)
@@ -2483,7 +2506,7 @@ static CURLMcode multi_socket(struct Curl_multi *multi, 
    return result;

  }

  if(s != CURL_SOCKET_TIMEOUT) {

    struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);

    struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s, FALSE);



    if(!entry)

      /* Unmatched socket, we can't act on it but we ignore this fact.  In
@@ -2493,18 +2516,20 @@ static CURLMcode multi_socket(struct Curl_multi *multi, 
         and just move on. */

      ;

    else {

      struct curl_llist *list = &entry->list;

      struct curl_llist_element *e;

      struct curl_llist_element *enext;

      struct curl_hash_iterator iter;

      struct curl_hash_element *he;

      SIGPIPE_VARIABLE(pipe_st);



      /* the socket can be shared by many transfers, iterate */

      for(e = list->head; e; e = enext) {

        data = (struct Curl_easy *)e->ptr;

      /* block this sockhash entry from being removed in a sub function called

         from here */

      entry->blocked = TRUE;

      DEBUGASSERT(!entry->removed);



        /* assign 'enext' here since the 'e' struct might be cleared

           further down in the singlesocket() call */

        enext = e->next;

      /* the socket can be shared by many transfers, iterate */

      Curl_hash_start_iterate(&entry->transfers, &iter);

      for(he = Curl_hash_next_element(&iter); he;

          he = Curl_hash_next_element(&iter)) {

        data = (struct Curl_easy *)he->ptr;



        DEBUGASSERT(data);

        DEBUGASSERT(data->magic == CURLEASY_MAGIC_NUMBER);
@@ -2529,6 +2554,10 @@ static CURLMcode multi_socket(struct Curl_multi *multi, 
            return result;

        }

      }

      if(entry->removed) {

        entry->blocked = FALSE; /* unblock */

        sh_delentry(entry, &multi->sockhash, s); /* delete for real */

      }



      /* Now we fall-through and do the timer-based stuff, since we don't want

         to force the user to have to deal with timeouts as long as at least
@@ -2976,7 +3005,7 @@ CURLMcode curl_multi_assign(struct Curl_multi *multi, curl_socket_t s, 
  if(multi->in_callback)

    return CURLM_RECURSIVE_API_CALL;



  there = sh_getentry(&multi->sockhash, s);

  there = sh_getentry(&multi->sockhash, s, FALSE);



  if(!there)

    return CURLM_BAD_SOCKET;
@@ -3065,7 +3094,7 @@ void Curl_multi_dump(struct Curl_multi *multi) 
              statename[data->mstate], data->numsocks);

      for(i = 0; i < data->numsocks; i++) {

        curl_socket_t s = data->sockets[i];

        struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);

        struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s, FALSE);



        fprintf(stderr, "%d ", (int)s);

        if(!entry) {

lib/url.c

+1 −8
Original line number Diff line number Diff line
@@ -1673,13 +1673,6 @@ static void free_idnconverted_hostname(struct hostname *host) 
#endif

}



static void llist_dtor(void *user, void *element)

{

  (void)user;

  (void)element;

  /* Do nothing */

}



/*

 * Allocate and initialize a new connectdata object.

 */
@@ -1791,7 +1784,7 @@ static struct connectdata *allocate_conn(struct Curl_easy *data) 
#endif



  /* Initialize the easy handle list */

  Curl_llist_init(&conn->easyq, (curl_llist_dtor) llist_dtor);

  Curl_llist_init(&conn->easyq, NULL);



#ifdef HAVE_GSSAPI

  conn->data_prot = PROT_CLEAR;

lib/urldata.h

+0 −2
Original line number Diff line number Diff line
@@ -1778,8 +1778,6 @@ struct Curl_easy { 


  struct connectdata *conn;

  struct curl_llist_element connect_queue;

  struct curl_llist_element sh_queue; /* list per Curl_sh_entry */

  struct Curl_sh_entry *sh_entry; /* the socket hash this was added to */

  struct curl_llist_element conn_queue; /* list per connectdata */



  CURLMstate mstate;  /* the handle's state */