Skip to content
Permalink
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
  • Loading branch information...
bagder committed Jun 5, 2019
1 parent fb2839d commit 094b5f3540fec1401f514bc470f11f441527d30a
Showing with 95 additions and 75 deletions.
  1. +94 −65 lib/multi.c
  2. +1 −8 lib/url.c
  3. +0 −2 lib/urldata.h
@@ -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,17 +281,17 @@ 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));
}
/* 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));
}

/*
@@ -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) {
@@ -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;
@@ -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 */

0 comments on commit 094b5f3

Please sign in to comment.
You can’t perform that action at this time.