From b05dbd778f16eb4c977e7769bd1ba03dd4e55b90 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 29 Aug 2017 17:29:10 +0200 Subject: ipcpd: Fix state in dht lookup at lookup_wait The correct state was returned but not set internally in the lookup_struct, causing lookup_wait to return a lookup in LU_PENDING if the wait timedout. This also unlocks the IRMd during register calls and fixes some cleanup if lookup_wait gets interrupted. Fixes #48 --- src/ipcpd/normal/dht.c | 39 ++++++++++++++++----------- src/irmd/main.c | 72 +++++++++++++++++++++++++++++++++----------------- 2 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/ipcpd/normal/dht.c b/src/ipcpd/normal/dht.c index b5bac913..a1d21ad7 100644 --- a/src/ipcpd/normal/dht.c +++ b/src/ipcpd/normal/dht.c @@ -837,6 +837,24 @@ static void lookup_new_addrs(struct lookup * lu, pthread_mutex_unlock(&lu->lock); } +static void lookup_set_state(struct lookup * lu, + enum lookup_state state) +{ + pthread_mutex_lock(&lu->lock); + + lu->state = state; + pthread_cond_signal(&lu->cond); + + pthread_mutex_unlock(&lu->lock); +} + +static void cleanup_wait(void * o) +{ + struct lookup * lu = (struct lookup *) o; + lookup_set_state(lu, LU_NULL); + lookup_destroy(lu); +} + static enum lookup_state lookup_wait(struct lookup * lu) { struct timespec timeo = {KAD_T_RESP, 0}; @@ -853,7 +871,7 @@ static enum lookup_state lookup_wait(struct lookup * lu) lu->state = LU_PENDING; pthread_cond_signal(&lu->cond); - pthread_cleanup_push((void (*)(void *)) lookup_destroy, (void *) lu); + pthread_cleanup_push(cleanup_wait, lu); while (lu->state == LU_PENDING && ret != -ETIMEDOUT) ret = -pthread_cond_timedwait(&lu->cond, &lu->lock, &abs); @@ -861,9 +879,9 @@ static enum lookup_state lookup_wait(struct lookup * lu) pthread_cleanup_pop(false); if (ret == -ETIMEDOUT) - state = LU_COMPLETE; - else - state = lu->state; + lu->state = LU_COMPLETE; + + state = lu->state; pthread_mutex_unlock(&lu->lock); @@ -1501,17 +1519,6 @@ static ssize_t kad_find(struct dht * dht, return sent; } -static void lookup_set_state(struct lookup * lu, - enum lookup_state state) -{ - pthread_mutex_lock(&lu->lock); - - lu->state = state; - pthread_cond_signal(&lu->cond); - - pthread_mutex_unlock(&lu->lock); -} - static struct lookup * kad_lookup(struct dht * dht, const uint8_t * id, enum kad_code code) @@ -1540,7 +1547,6 @@ static struct lookup * kad_lookup(struct dht * dht, pthread_rwlock_wrlock(&dht->lock); list_del(&lu->next); pthread_rwlock_unlock(&dht->lock); - lu->state = LU_COMPLETE; return lu; } @@ -1554,6 +1560,7 @@ static struct lookup * kad_lookup(struct dht * dht, pthread_rwlock_wrlock(&dht->lock); list_del(&lu->next); pthread_rwlock_unlock(&dht->lock); + lookup_set_state(lu, LU_COMPLETE); return lu; } diff --git a/src/irmd/main.c b/src/irmd/main.c index 09a76214..1d62de85 100644 --- a/src/irmd/main.c +++ b/src/irmd/main.c @@ -922,6 +922,8 @@ static int name_reg(const char * name, for (i = 0; i < len; ++i) { uint8_t * hash; + pid_t api; + size_t len; if (wildcard_match(difs[i], e->dif_name)) continue; @@ -932,23 +934,33 @@ static int name_reg(const char * name, str_hash(e->dir_hash_algo, hash, name); - if (ipcp_reg(e->api, hash, IPCP_HASH_LEN(e))) { + api = e->api; + len = IPCP_HASH_LEN(e); + + pthread_rwlock_unlock(&irmd.reg_lock); + + if (ipcp_reg(api, hash, len)) { log_err("Could not register " HASH_FMT - " in DIF %s (IPCP %d).", - HASH_VAL(hash), e->dif_name, e->api); - } else { - if (registry_add_name_to_dif(&irmd.registry, - name, - e->dif_name, - e->type) < 0) - log_warn("Registered unbound name %s. " - "Registry may be corrupt.", - name); - log_info("Registered %s in %s as " HASH_FMT ".", - name, e->dif_name, HASH_VAL(hash)); - ++ret; + " with IPCP %d.", + HASH_VAL(hash), api); + pthread_rwlock_wrlock(&irmd.reg_lock); + free(hash); + break; } + pthread_rwlock_wrlock(&irmd.reg_lock); + + if (registry_add_name_to_dif(&irmd.registry, + name, + e->dif_name, + e->type) < 0) + log_warn("Registered unbound name %s. " + "Registry may be corrupt.", + name); + log_info("Registered %s in %s as " HASH_FMT ".", + name, e->dif_name, HASH_VAL(hash)); + ++ret; + free(hash); } } @@ -982,6 +994,8 @@ static int name_unreg(const char * name, for (i = 0; i < len; ++i) { uint8_t * hash; + pid_t api; + size_t len; if (wildcard_match(difs[i], e->dif_name)) continue; @@ -992,18 +1006,28 @@ static int name_unreg(const char * name, str_hash(e->dir_hash_algo, hash, name); - if (ipcp_unreg(e->api, hash, IPCP_HASH_LEN(e))) { - log_err("Could not unregister %s in DIF %s.", - name, e->dif_name); - } else { - registry_del_name_from_dif(&irmd.registry, - name, - e->dif_name); - log_info("Unregistered %s from %s.", - name, e->dif_name); - ++ret; + api = e->api; + len = IPCP_HASH_LEN(e); + + pthread_rwlock_unlock(&irmd.reg_lock); + + if (ipcp_unreg(api, hash, len)) { + log_err("Could not unregister %s with IPCP %d.", + name, api); + pthread_rwlock_wrlock(&irmd.reg_lock); + free(hash); + break; } + pthread_rwlock_wrlock(&irmd.reg_lock); + + registry_del_name_from_dif(&irmd.registry, + name, + e->dif_name); + log_info("Unregistered %s from %s.", + name, e->dif_name); + ++ret; + free(hash); } } -- cgit v1.2.3