From 39f83b080fcc13d4de36db5510694c6a1f55b8d1 Mon Sep 17 00:00:00 2001
From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Fri, 3 May 2019 13:31:23 -0500
Subject: [PATCH] Another batch of memory management polish.

---
 src/common.c         | 22 +++++++++++++++++++++-
 src/common.h         |  4 ++++
 src/rtr/rtr.c        | 12 +++++++-----
 src/updates_daemon.c |  8 ++++----
 4 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/src/common.c b/src/common.c
index 29a8913a..e210c1a1 100644
--- a/src/common.c
+++ b/src/common.c
@@ -1,7 +1,6 @@
 #include "common.h"
 
 #include <errno.h>
-#include <pthread.h>
 #include <stdlib.h>
 #include "log.h"
 
@@ -64,3 +63,24 @@ rwlock_unlock(pthread_rwlock_t *lock)
 		exit(error);
 	}
 }
+
+int
+close_thread(pthread_t thread, char const *what)
+{
+	int error;
+
+	error = pthread_cancel(thread);
+	if (error && error != ESRCH) {
+		pr_crit("pthread_cancel() threw %d on the '%s' thread.",
+		    error, what);
+		return error;
+	}
+
+	error = pthread_join(thread, NULL);
+	if (error)
+		pr_crit("pthread_join() threw %d on the '%s' thread.",
+		    error, what);
+
+	return error;
+}
+
diff --git a/src/common.h b/src/common.h
index a5c5c596..469a4c85 100644
--- a/src/common.h
+++ b/src/common.h
@@ -1,6 +1,7 @@
 #ifndef SRC_RTR_COMMON_H_
 #define SRC_RTR_COMMON_H_
 
+#include <pthread.h>
 #include <semaphore.h>
 
 /* "I think that this is not supposed to be implemented." */
@@ -31,4 +32,7 @@ int rwlock_read_lock(pthread_rwlock_t *);
 void rwlock_write_lock(pthread_rwlock_t *);
 void rwlock_unlock(pthread_rwlock_t *);
 
+/** Also boilerplate. */
+int close_thread(pthread_t thread, char const *what);
+
 #endif /* SRC_RTR_COMMON_H_ */
diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c
index 0f4b44eb..b2cf7fdc 100644
--- a/src/rtr/rtr.c
+++ b/src/rtr/rtr.c
@@ -302,7 +302,7 @@ init_signal_handler(void)
 	return error;
 }
 
-/* Wait for threads to end gracefully */
+/* Terminates client threads as gracefully as I know how to. */
 static void
 wait_threads(void)
 {
@@ -312,10 +312,12 @@ wait_threads(void)
 	while (!SLIST_EMPTY(&threads)) {
 		ptr = SLIST_FIRST(&threads);
 		SLIST_REMOVE_HEAD(&threads, next);
-		/* TODO interrupt then join? Is this legal? */
-		pthread_kill(ptr->tid, SIGINT);
-		pthread_join(ptr->tid, NULL);
-		free(ptr);
+		/*
+		 * If the close fails, the thread might still be using the
+		 * thread_param variables, so leak instead.
+		 */
+		if (close_thread(ptr->tid, "Client") == 0)
+			free(ptr);
 	}
 }
 
diff --git a/src/updates_daemon.c b/src/updates_daemon.c
index 4572d90e..b7713d50 100644
--- a/src/updates_daemon.c
+++ b/src/updates_daemon.c
@@ -1,12 +1,12 @@
 #include "updates_daemon.h"
 
 #include <errno.h>
-#include <pthread.h>
 #include <stdbool.h>
 #include <unistd.h>
 
+#include "common.h"
 #include "config.h"
-#include "log.h" /* TODO delete me probably */
+#include "log.h"
 #include "notify.h"
 #include "object/tal.h"
 #include "rtr/db/vrps.h"
@@ -57,6 +57,6 @@ updates_daemon_start(void)
 void
 updates_daemon_destroy(void)
 {
-	pthread_cancel(thread);
-	pthread_join(thread, NULL);
+	/* Not much to do with the error code. */
+	close_thread(thread, "Validation");
 }
-- 
2.47.3