From ff7e5c12c9d50fb2ba79d045e99a7ede602ed6f3 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 27 Oct 2016 17:57:16 +0200 Subject: lib: Clean up flow if alloc_res fails If alloc_res fails all resources will be cleaned, except for the assigned flow_descriptor, which must be released with a dealloc call. Calling dealloc after a failed flow_alloc will not try to destroy the already cleaned up flow in the IRMd and IPCPs. Also fixes some memleaks in the oping client. --- src/lib/dev.c | 109 ++++++++++++++++++++++++----------------- src/tools/oping/oping_client.c | 101 ++++++++++++++++++++++++-------------- 2 files changed, 129 insertions(+), 81 deletions(-) diff --git a/src/lib/dev.c b/src/lib/dev.c index a0c47403..3d527943 100644 --- a/src/lib/dev.c +++ b/src/lib/dev.c @@ -189,6 +189,37 @@ static int api_announce(char * ap_name) return ret; } +static void reset_flow(int fd) +{ + assert (!(fd < 0)); + + if (ai.flows[fd].port_id != -1) + port_destroy(&ai.ports[ai.flows[fd].port_id]); + + ai.flows[fd].port_id = -1; + if (ai.flows[fd].rx_rb != NULL) { + shm_rbuff_close(ai.flows[fd].rx_rb); + ai.flows[fd].rx_rb = NULL; + } + if (ai.flows[fd].tx_rb != NULL) { + shm_rbuff_close(ai.flows[fd].tx_rb); + ai.flows[fd].tx_rb = NULL; + } + + if (ai.flows[fd].set != NULL) { + shm_flow_set_close(ai.flows[fd].set); + ai.flows[fd].set = NULL; + } + + ai.flows[fd].oflags = 0; + ai.flows[fd].api = -1; + + if (ai.flows[fd].timeout != NULL) { + free(ai.flows[fd].timeout); + ai.flows[fd].timeout = NULL; + } +} + int ap_init(char * ap_name) { int i = 0; @@ -288,13 +319,8 @@ void ap_fini() ssize_t idx; while ((idx = shm_rbuff_read(ai.flows[i].rx_rb)) >= 0) shm_rdrbuff_remove(ai.rdrb, idx); - shm_rbuff_close(ai.flows[i].rx_rb); - shm_rbuff_close(ai.flows[i].tx_rb); - shm_flow_set_close(ai.flows[i].set); } - - if (ai.flows[i].timeout != NULL) - free(ai.flows[i].timeout); + reset_flow(i); } for (i = 0; i < IRMD_MAX_FLOWS; ++i) { @@ -315,7 +341,6 @@ void ap_fini() pthread_rwlock_destroy(&ai.data_lock); } - int flow_accept(char ** ae_name, struct qos_spec * qos) { irm_msg_t msg = IRM_MSG__INIT; @@ -355,6 +380,7 @@ int flow_accept(char ** ae_name, struct qos_spec * qos) ai.flows[fd].rx_rb = shm_rbuff_open(ai.api, recv_msg->port_id); if (ai.flows[fd].rx_rb == NULL) { + reset_flow(fd); bmp_release(ai.fds, fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); @@ -364,9 +390,8 @@ int flow_accept(char ** ae_name, struct qos_spec * qos) ai.flows[fd].set = shm_flow_set_open(recv_msg->api); if (ai.flows[fd].set == NULL) { + reset_flow(fd); bmp_release(ai.fds, fd); - shm_rbuff_close(ai.flows[fd].rx_rb); - shm_rbuff_close(ai.flows[fd].tx_rb); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); irm_msg__free_unpacked(recv_msg, NULL); @@ -377,9 +402,7 @@ int flow_accept(char ** ae_name, struct qos_spec * qos) if (ae_name != NULL) { *ae_name = strdup(recv_msg->ae_name); if (*ae_name == NULL) { - shm_rbuff_close(ai.flows[fd].tx_rb); - shm_rbuff_close(ai.flows[fd].tx_rb); - shm_flow_set_close(ai.flows[fd].set); + reset_flow(fd); bmp_release(ai.fds, fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); @@ -452,6 +475,7 @@ int flow_alloc_resp(int fd, int response) ai.flows[fd].tx_rb = shm_rbuff_open(ai.flows[fd].api, ai.flows[fd].port_id); if (ai.flows[fd].tx_rb == NULL) { + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -516,6 +540,7 @@ int flow_alloc(char * dst_name, char * src_ae_name, struct qos_spec * qos) ai.flows[fd].api = recv_msg->api; ai.flows[fd].rx_rb = shm_rbuff_open(ai.api, recv_msg->port_id); if (ai.flows[fd].rx_rb == NULL) { + reset_flow(fd); bmp_release(ai.fds, fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); @@ -567,7 +592,7 @@ int flow_alloc_res(int fd) ai.flows[fd].set = shm_flow_set_open(ai.flows[fd].api); if (ai.flows[fd].set == NULL) { - shm_rbuff_close(ai.flows[fd].tx_rb); + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -577,19 +602,28 @@ int flow_alloc_res(int fd) pthread_rwlock_unlock(&ai.data_lock); recv_msg = send_recv_irm_msg_b(&msg); - if (recv_msg == NULL) { - return -1; - } + if (recv_msg == NULL) + result = -1; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); - return -1; + result = -1; } result = recv_msg->result; irm_msg__free_unpacked(recv_msg, NULL); + if (result) { + pthread_rwlock_rdlock(&ai.data_lock); + pthread_rwlock_wrlock(&ai.flows_lock); + + reset_flow(fd); + + pthread_rwlock_unlock(&ai.flows_lock); + pthread_rwlock_unlock(&ai.data_lock); + } + return result; } @@ -598,6 +632,9 @@ int flow_dealloc(int fd) irm_msg_t msg = IRM_MSG__INIT; irm_msg_t * recv_msg = NULL; + if (fd < 0) + return -EINVAL; + msg.code = IRM_MSG_CODE__IRM_FLOW_DEALLOC; msg.has_port_id = true; msg.has_api = true; @@ -607,9 +644,10 @@ int flow_dealloc(int fd) pthread_rwlock_wrlock(&ai.flows_lock); if (ai.flows[fd].port_id < 0) { + bmp_release(ai.fds, fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); - return -ENOTALLOC; + return 0; } msg.port_id = ai.flows[fd].port_id; @@ -632,22 +670,7 @@ int flow_dealloc(int fd) pthread_rwlock_rdlock(&ai.data_lock); pthread_rwlock_wrlock(&ai.flows_lock); - port_destroy(&ai.ports[msg.port_id]); - - ai.flows[fd].port_id = -1; - shm_rbuff_close(ai.flows[fd].rx_rb); - ai.flows[fd].rx_rb = NULL; - shm_rbuff_close(ai.flows[fd].tx_rb); - ai.flows[fd].tx_rb = NULL; - ai.flows[fd].oflags = 0; - ai.flows[fd].api = -1; - shm_flow_set_close(ai.flows[fd].set); - if (ai.flows[fd].timeout != NULL) { - free(ai.flows[fd].timeout); - ai.flows[fd].timeout = NULL; - } - - bmp_release(ai.fds, fd); + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); @@ -865,7 +888,8 @@ struct fqueue * fqueue_create() void fqueue_destroy(struct fqueue * fq) { if (fq == NULL) - return + return; + free(fq); } @@ -1003,7 +1027,7 @@ int np1_flow_alloc(pid_t n_api, int port_id) ai.flows[fd].rx_rb = shm_rbuff_open(ai.api, port_id); if (ai.flows[fd].rx_rb == NULL) { - bmp_release(ai.fds, fd); + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -1056,8 +1080,7 @@ int np1_flow_resp(pid_t n_api, int port_id) ai.flows[fd].tx_rb = shm_rbuff_open(n_api, port_id); if (ai.flows[fd].tx_rb == NULL) { - ai.flows[fd].port_id = -1; - shm_rbuff_close(ai.flows[fd].rx_rb); + reset_flow(fd); port_destroy(&ai.ports[port_id]); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); @@ -1066,10 +1089,7 @@ int np1_flow_resp(pid_t n_api, int port_id) ai.flows[fd].set = shm_flow_set_open(n_api); if (ai.flows[fd].set == NULL) { - shm_rbuff_close(ai.flows[fd].tx_rb); - ai.flows[fd].port_id = -1; - shm_rbuff_close(ai.flows[fd].rx_rb); - port_destroy(&ai.ports[port_id]); + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -1156,8 +1176,7 @@ int ipcp_flow_req_arr(pid_t api, char * dst_name, char * src_ae_name) ai.flows[fd].rx_rb = shm_rbuff_open(ai.api, port_id); if (ai.flows[fd].rx_rb == NULL) { - ai.flows[fd].port_id = -1; - port_destroy(&ai.ports[port_id]); + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -1209,6 +1228,7 @@ int ipcp_flow_alloc_reply(int fd, int response) ai.flows[fd].tx_rb = shm_rbuff_open(ai.flows[fd].api, ai.flows[fd].port_id); if (ai.flows[fd].tx_rb == NULL) { + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; @@ -1216,6 +1236,7 @@ int ipcp_flow_alloc_reply(int fd, int response) ai.flows[fd].set = shm_flow_set_open(ai.flows[fd].api); if (ai.flows[fd].set == NULL) { + reset_flow(fd); pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); return -1; diff --git a/src/tools/oping/oping_client.c b/src/tools/oping/oping_client.c index 85cb2880..1ce96a75 100644 --- a/src/tools/oping/oping_client.c +++ b/src/tools/oping/oping_client.c @@ -162,6 +162,51 @@ void * writer(void * o) return (void *) 0; } +static int client_init(void) +{ + client.flows = flow_set_create(); + if (client.flows == NULL) + return -ENOMEM; + + client.fq = fqueue_create(); + if (client.fq == NULL) { + flow_set_destroy(client.flows); + return -ENOMEM; + } + + client.times = malloc(sizeof(struct timespec) * client.count); + if (client.times == NULL) { + flow_set_destroy(client.flows); + fqueue_destroy(client.fq); + pthread_mutex_unlock(&client.lock); + return -ENOMEM; + } + + client.sent = 0; + client.rcvd = 0; + client.rtt_min = FLT_MAX; + client.rtt_max = 0; + client.rtt_avg = 0; + client.rtt_m2 = 0; + + pthread_mutex_init(&client.lock, NULL); + pthread_mutex_lock(&client.lock); + + return 0; +} + +void client_fini(void) +{ + if (client.flows != NULL) + flow_set_destroy(client.flows); + + if (client.fq != NULL) + fqueue_destroy(client.fq); + + if (client.times != NULL) + free(client.times); +} + int client_main(void) { struct sigaction sig_act; @@ -171,18 +216,27 @@ int client_main(void) int fd; - client.flows = flow_set_create(); - if (client.flows == NULL) + memset(&sig_act, 0, sizeof sig_act); + sig_act.sa_sigaction = &shutdown_client; + sig_act.sa_flags = 0; + + if (sigaction(SIGINT, &sig_act, NULL) || + sigaction(SIGTERM, &sig_act, NULL) || + sigaction(SIGHUP, &sig_act, NULL) || + sigaction(SIGPIPE, &sig_act, NULL)) { + printf("Failed to install sighandler.\n"); return -1; + } - client.fq = fqueue_create(); - if (client.fq == NULL) { - flow_set_destroy(client.flows); + if (client_init()) { + printf("Failed to initialize client.\n"); return -1; } fd = flow_alloc(client.s_apn, NULL, NULL); if (fd < 0) { + flow_set_destroy(client.flows); + fqueue_destroy(client.fq); printf("Failed to allocate flow.\n"); return -1; } @@ -191,36 +245,12 @@ int client_main(void) if (flow_alloc_res(fd)) { printf("Flow allocation refused.\n"); + flow_set_del(client.flows, fd); flow_dealloc(fd); + client_fini(); return -1; } - memset(&sig_act, 0, sizeof sig_act); - sig_act.sa_sigaction = &shutdown_client; - sig_act.sa_flags = 0; - - if (sigaction(SIGINT, &sig_act, NULL) || - sigaction(SIGTERM, &sig_act, NULL) || - sigaction(SIGHUP, &sig_act, NULL) || - sigaction(SIGPIPE, &sig_act, NULL)) { - printf("Failed to install sighandler.\n"); - return -1; - } - - pthread_mutex_init(&client.lock, NULL); - pthread_mutex_lock(&client.lock); - client.sent = 0; - client.rcvd = 0; - client.rtt_min = FLT_MAX; - client.rtt_max = 0; - client.rtt_avg = 0; - client.rtt_m2 = 0; - client.times = malloc(sizeof(struct timespec) * client.count); - if (client.times == NULL) { - pthread_mutex_unlock(&client.lock); - return -ENOMEM; - } - pthread_mutex_unlock(&client.lock); clock_gettime(CLOCK_REALTIME, &tic); @@ -253,14 +283,11 @@ int client_main(void) printf("NaN ms\n"); } - pthread_mutex_lock(&client.lock); - free(client.times); - flow_set_destroy(client.flows); - fqueue_destroy(client.fq); - pthread_mutex_unlock(&client.lock); - pthread_mutex_destroy(&client.lock); + flow_set_del(client.flows, fd); flow_dealloc(fd); + client_fini(); + return 0; } -- cgit v1.2.3