From: pcarana Date: Wed, 27 Mar 2019 17:41:34 +0000 (-0600) Subject: Remove some memleaks reported by Valgrind X-Git-Tag: v0.0.2~52^2~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ecb093f07a26f8a348b87cca8ae62b4dad1ae3f6;p=thirdparty%2FFORT-validator.git Remove some memleaks reported by Valgrind -Make threads joinable and implement a destroy function used by the main thread (here all the threads are joined). -Start the updates_daemon until the server socket is correctly binded. -Valgrind reports some leaks on abnormal termination, so these will be handled later. --- diff --git a/src/main.c b/src/main.c index bf1be53f..5496b585 100644 --- a/src/main.c +++ b/src/main.c @@ -7,7 +7,6 @@ #include "clients.h" #include "configuration.h" #include "csv.h" -#include "updates_daemon.h" #include "vrps.h" /* @@ -65,12 +64,9 @@ main(int argc, char *argv[]) if (err) goto end3; - err = updates_daemon_init(); - if (err) - goto end3; - err = rtr_listen(); + rtr_cleanup(); end3: clients_db_destroy(); end2: diff --git a/src/rtr/pdu.c b/src/rtr/pdu.c index a1db16ac..770e52a8 100644 --- a/src/rtr/pdu.c +++ b/src/rtr/pdu.c @@ -178,7 +178,7 @@ error_report_destroy(void *pdu_void) warnx("Unknown PDU type (%u).", sub_hdr->pdu_type); free(pdu->error_message); - free(pdu); + free(pdu_void); } #define DEFINE_METADATA(name, dtor) \ diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index 7a1da410..85b5cdd4 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -10,7 +10,9 @@ #include #include #include +#include +#include "../updates_daemon.h" #include "clients.h" #include "configuration.h" #include "err_pdu.h" @@ -19,6 +21,15 @@ /* TODO Support both RTR v0 an v1 */ #define RTR_VERSION_SUPPORTED RTR_V0 +volatile bool loop; + +struct thread_node { + pthread_t tid; + SLIST_ENTRY(thread_node) next; +}; + +SLIST_HEAD(thread_list, thread_node) threads; + /* * Creates the socket that will stay put and wait for new connections started * from the clients. @@ -31,7 +42,8 @@ create_server_socket(void) addr = config_get_server_addrinfo(); for (; addr != NULL; addr = addr->ai_next) { - printf("Attempting to bind socket to address '%s', port '%s'.\n", + printf( + "Attempting to bind socket to address '%s', port '%s'.\n", (addr->ai_canonname != NULL) ? addr->ai_canonname : "any", config_get_server_port()); @@ -121,9 +133,8 @@ client_thread_cb(void *param_void) u_int8_t rtr_version; memcpy(¶m, param_void, sizeof(param)); - free(param_void); /* Ha. */ - while (true) { /* For each PDU... */ + while (loop) { /* For each PDU... */ err = pdu_load(param.client_fd, &pdu, &meta, &rtr_version); if (err) return NULL; @@ -166,11 +177,12 @@ client_thread_cb(void *param_void) static int handle_client_connections(int server_fd) { - int client_fd; struct sockaddr_storage client_addr; + struct thread_param arg; + struct thread_node *new_thread; socklen_t sizeof_client_addr; - struct thread_param *arg; - pthread_t thread; + pthread_attr_t attr; + int client_fd; listen(server_fd, config_get_server_queue()); @@ -190,29 +202,34 @@ handle_client_connections(int server_fd) /* * Note: My gut says that errors from now on (even the unknown - * ones) should be treated as temporary; maybe the next accept() - * will work. + * ones) should be treated as temporary; maybe the next + * accept() will work. * So don't interrupt the thread when this happens. */ - arg = malloc(sizeof(struct thread_param)); - if (arg == NULL) { - warn("Thread parameter allocation failure"); + new_thread = malloc(sizeof(struct thread_node)); + if (new_thread == NULL) { + warnx("Couldn't create thread struct"); + close(client_fd); continue; } - arg->client_fd = client_fd; - arg->client_addr = client_addr; - errno = pthread_create(&thread, NULL, client_thread_cb, arg); + arg.client_fd = client_fd; + arg.client_addr = client_addr; + + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); + errno = pthread_create(&new_thread->tid, &attr, + client_thread_cb, &arg); + pthread_attr_destroy(&attr); if (errno) { warn("Could not spawn the client's thread"); - free(arg); + free(new_thread); close(client_fd); continue; } - /* BTW: The thread will be responsible for releasing @arg. */ - pthread_detach(thread); + SLIST_INSERT_HEAD(&threads, new_thread, next); } while (true); @@ -229,10 +246,37 @@ int rtr_listen(void) { int server_fd; /* "file descriptor" */ + int error; server_fd = create_server_socket(); if (server_fd < 0) return server_fd; + /* Server ready, start updates thread */ + error = updates_daemon_start(); + if (error) + return error; + + /* Init global vars */ + loop = true; + SLIST_INIT(&threads); + return handle_client_connections(server_fd); } + +void +rtr_cleanup(void) +{ + struct thread_node *ptr; + + updates_daemon_destroy(); + + /* Wait for threads to end gracefully */ + loop = false; + while (!SLIST_EMPTY(&threads)) { + ptr = SLIST_FIRST(&threads); + SLIST_REMOVE_HEAD(&threads, next); + pthread_join(ptr->tid, NULL); + free(ptr); + } +} diff --git a/src/rtr/rtr.h b/src/rtr/rtr.h index 6cae8eab..0dfc1111 100644 --- a/src/rtr/rtr.h +++ b/src/rtr/rtr.h @@ -5,6 +5,7 @@ __BEGIN_DECLS int rtr_listen(void); +void rtr_cleanup(void); __END_DECLS #endif /* RTR_RTR_H_ */ diff --git a/src/updates_daemon.c b/src/updates_daemon.c index f105c47d..3206bbaf 100644 --- a/src/updates_daemon.c +++ b/src/updates_daemon.c @@ -10,6 +10,8 @@ #include "configuration.h" #include "notify.h" +pthread_t thread; + static void * check_vrps_updates(void *param_void) { int error; @@ -31,13 +33,26 @@ sleep: } int -updates_daemon_init(void) { - pthread_t thread; +updates_daemon_start(void) +{ + pthread_attr_t attr; + + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); errno = pthread_create(&thread, NULL, check_vrps_updates, NULL); + pthread_attr_destroy(&attr); if (errno) { warn("Could not spawn the update daemon thread"); - return errno; + return -errno; } - pthread_detach(thread); + return 0; } + +void +updates_daemon_destroy(void) +{ + void *ptr = NULL; + pthread_cancel(thread); + pthread_join(thread, &ptr); +} diff --git a/src/updates_daemon.h b/src/updates_daemon.h index 47124df2..ea269e5b 100644 --- a/src/updates_daemon.h +++ b/src/updates_daemon.h @@ -1,6 +1,7 @@ #ifndef SRC_UPDATES_DAEMON_H_ #define SRC_UPDATES_DAEMON_H_ -int updates_daemon_init(void); +int updates_daemon_start(void); +void updates_daemon_destroy(void); #endif /* SRC_UPDATES_DAEMON_H_ */