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 bf1be53f3f4f14ee4b83b561a74327f9ec993f78..5496b58592b1428155ca414b142a80b847cd8ae0 100644 (file)
--- 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 a1db16ac246fa236956b723e09a6e9937ca9f29f..770e52a8be78da2458372d1b392467616c275bb9 100644 (file)
--- 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 7a1da41034279a8d98eec4dc7e86e84752485481..85b5cdd49d301f9252803a17d52139271f2e8447 100644 (file)
--- a/src/rtr/rtr.c
+++ b/src/rtr/rtr.c
@@ -10,7 +10,9 @@
 #include <string.h>
 #include <unistd.h>
 #include <arpa/inet.h>
+#include <sys/queue.h>
 
+#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(&param, 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 6cae8eabc5fd1906127b92921b684eefc3f283b2..0dfc1111455bfea04f3d0655a4af60566bbe578d 100644 (file)
--- 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 f105c47d826c7c246a935da0fccd47b9180edeaa..3206bbafd0cd4b80d09052d4f461a7b88fc33943 100644 (file)
--- 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 47124df210022845b53f7eb40060353c189119e3..ea269e5bc8eb222d39c22fbdef7515eecf65023e 100644 (file)
--- 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_ */