From 3d5f7e76735f51930ea804c6ac6a085abb991b53 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 09:55:57 +0100
Subject: [PATCH 1/8] Do not terminate threads which may not be running.
 They'll be cleaned up at process termination anyway.

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/args.c              |  3 +++
 src/dnsmasq_interface.c | 12 ++++++------
 src/main.c              |  5 -----
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/args.c b/src/args.c
index fd786dca3..e988fc882 100644
--- a/src/args.c
+++ b/src/args.c
@@ -188,6 +188,9 @@ void parse_args(int argc, char* argv[])
 			argv_dnsmasq[1] = "-d";
 		}
 
+		// Full start FTL but shut down immediately once everything is up
+		// This ensures we'd catch any dnsmasq config errors,
+		// incorrect file permissions, etc.
 		if(strcmp(argv[i], "test") == 0)
 		{
 			killed = 1;
diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c
index d5906a3d3..54151abca 100644
--- a/src/dnsmasq_interface.c
+++ b/src/dnsmasq_interface.c
@@ -1740,12 +1740,12 @@ static void save_reply_type(const unsigned int flags, const union all_addr *addr
 	                            query->response;
 }
 
-pthread_t telnet_listenthreadv4;
-pthread_t telnet_listenthreadv6;
-pthread_t socket_listenthread;
-pthread_t DBthread;
-pthread_t GCthread;
-pthread_t DNSclientthread;
+pthread_t telnet_listenthreadv4 = 0u;
+pthread_t telnet_listenthreadv6 = 0u;
+pthread_t socket_listenthread = 0u;
+pthread_t DBthread = 0u;
+pthread_t GCthread = 0u;
+pthread_t DNSclientthread = 0u;
 
 void FTL_fork_and_bind_sockets(struct passwd *ent_pw)
 {
diff --git a/src/main.c b/src/main.c
index bbdad82ac..a38743566 100644
--- a/src/main.c
+++ b/src/main.c
@@ -98,11 +98,6 @@ int main (int argc, char* argv[])
 
 	logg("Shutting down...");
 
-	// Cancel active threads as we don't need them any more
-	if(ipv4telnet) pthread_cancel(telnet_listenthreadv4);
-	if(ipv6telnet) pthread_cancel(telnet_listenthreadv6);
-	pthread_cancel(socket_listenthread);
-
 	// Save new queries to database (if database is used)
 	if(config.DBexport)
 	{

From b475e5be1f845b52bf3ee9c9b2ca04f22dad4a13 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 11:46:36 +0100
Subject: [PATCH 2/8] Ensure we clean up always behind us. Also when FTL
 crashes

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/daemon.c  | 36 +++++++++++++++++++++++++++++++++++-
 src/daemon.h  |  5 ++---
 src/main.c    | 25 ++-----------------------
 src/signals.c |  2 +-
 4 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/src/daemon.c b/src/daemon.c
index 2d72d20b2..214568b7f 100644
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -14,6 +14,12 @@
 #include "log.h"
 // sleepms()
 #include "timers.h"
+// close_telnet_socket()
+#include "api/socket.h"
+// gravityDB_close()
+#include "database/gravity-db.h"
+// destroy_shmem()
+#include "shmem.h"
 
 bool resolver_ready = false;
 
@@ -99,7 +105,7 @@ void savepid(void)
 	logg("PID of FTL process: %i", (int)pid);
 }
 
-void removepid(void)
+static void removepid(void)
 {
 	FILE *f;
 	if((f = fopen(FTLfiles.pid, "w+")) == NULL)
@@ -108,6 +114,7 @@ void removepid(void)
 		return;
 	}
 	fclose(f);
+	unlink(FTLfiles.pid);
 }
 
 char *getUserName(void)
@@ -158,3 +165,30 @@ pid_t FTL_gettid(void)
 	return -1;
 #endif // SYS_gettid
 }
+
+// Clean up on exit
+void cleanup(const int ret)
+{
+	// Close sockets and delete Unix socket file handle
+	close_telnet_socket();
+	close_unix_socket(true);
+
+	// Empty API port file, port 0 = truncate file
+	saveport(0);
+
+	// Close gravity database connection
+	gravityDB_close();
+
+	// Remove shared memory objects
+	// Important: This invalidated all objects such as
+	//            counters-> ... Do this last when
+	//            terminating in main.c !
+	destroy_shmem();
+
+	//Remove PID file
+	removepid();
+
+	char buffer[42] = { 0 };
+	format_time(buffer, 0, timer_elapsed_msec(EXIT_TIMER));
+	logg("########## FTL terminated after%s (code %i)! ##########", buffer, ret);
+}
diff --git a/src/daemon.h b/src/daemon.h
index 939a5fb3d..5c2f4fb92 100644
--- a/src/daemon.h
+++ b/src/daemon.h
@@ -12,11 +12,10 @@
 
 void go_daemon(void);
 void savepid(void);
-char * getUserName(void);
-void removepid(void);
+char *getUserName(void);
 void delay_startup(void);
 bool is_fork(const pid_t mpid, const pid_t pid) __attribute__ ((const));
-
+void cleanup(const int ret);
 
 #include <sys/syscall.h>
 #include <unistd.h>
diff --git a/src/main.c b/src/main.c
index a38743566..fe4de7c9f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -11,7 +11,6 @@
 #include "FTL.h"
 #include "daemon.h"
 #include "log.h"
-#include "api/socket.h"
 #include "setupVars.h"
 #include "args.h"
 #include "config.h"
@@ -20,9 +19,9 @@
 #include "main.h"
 #include "signals.h"
 #include "regex_r.h"
+// init_shmem()
 #include "shmem.h"
 #include "capabilities.h"
-#include "database/gravity-db.h"
 #include "timers.h"
 #include "procps.h"
 
@@ -105,27 +104,7 @@ int main (int argc, char* argv[])
 		logg("Finished final database update");
 	}
 
-	// Close sockets and delete Unix socket file handle
-	close_telnet_socket();
-	close_unix_socket(true);
+	cleanup(exit_code);
 
-	// Empty API port file, port 0 = truncate file
-	saveport(0);
-
-	// Close gravity database connection
-	gravityDB_close();
-
-	// Remove shared memory objects
-	// Important: This invalidated all objects such as
-	//            counters-> ... Do this last when
-	//            terminating in main.c !
-	destroy_shmem();
-
-	//Remove PID file
-	removepid();
-
-	char buffer[42] = { 0 };
-	format_time(buffer, 0, timer_elapsed_msec(EXIT_TIMER));
-	logg("########## FTL terminated after%s! ##########", buffer);
 	return exit_code;
 }
diff --git a/src/signals.c b/src/signals.c
index 588e89e36..9a662e020 100644
--- a/src/signals.c
+++ b/src/signals.c
@@ -237,7 +237,7 @@ static void __attribute__((noreturn)) signal_handler(int sig, siginfo_t *si, voi
 	else
 	{
 		// This is the main process
-		logg("FTL terminated!");
+		cleanup(EXIT_FAILURE);
 	}
 
 	// Terminate process indicating failure

From b6fe3379b0a52bd09bc6c62ed547138f3d27ca0d Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 11:50:32 +0100
Subject: [PATCH 3/8] Also clean up when crashing

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/api/socket.c | 15 ++++++++-------
 src/daemon.c     | 17 ++++++++---------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/api/socket.c b/src/api/socket.c
index eba21f267..438dcf908 100644
--- a/src/api/socket.c
+++ b/src/api/socket.c
@@ -281,23 +281,24 @@ static int listener(const int sockfd, const char type)
 void close_telnet_socket(void)
 {
 	// Using global variable here
-	if(telnetfd4)
+	if(telnetfd4 != 0)
 		close(telnetfd4);
-	if(telnetfd6)
+	if(telnetfd6 != 0)
 		close(telnetfd6);
 }
 
 void close_unix_socket(bool unlink_file)
 {
+	// Using global variable here
+	if(sock_avail != 0)
+		close(socketfd);
+
+	// The process has to take care of unlinking the socket file description
+	// on exit
 	if(unlink_file)
 	{
-		// The process has to take care of unlinking the socket file description on exit
 		unlink(FTLfiles.socketfile);
 	}
-
-	// Using global variable here
-	if(sock_avail)
-		close(socketfd);
 }
 
 static void *telnet_connection_handler_thread(void *socket_desc)
diff --git a/src/daemon.c b/src/daemon.c
index 214568b7f..93cd73c33 100644
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -108,13 +108,12 @@ void savepid(void)
 static void removepid(void)
 {
 	FILE *f;
-	if((f = fopen(FTLfiles.pid, "w+")) == NULL)
+	if((f = fopen(FTLfiles.pid, "w")) == NULL)
 	{
 		logg("WARNING: Unable to empty PID file");
 		return;
 	}
 	fclose(f);
-	unlink(FTLfiles.pid);
 }
 
 char *getUserName(void)
@@ -169,6 +168,9 @@ pid_t FTL_gettid(void)
 // Clean up on exit
 void cleanup(const int ret)
 {
+	// Close gravity database connection
+	gravityDB_close();
+
 	// Close sockets and delete Unix socket file handle
 	close_telnet_socket();
 	close_unix_socket(true);
@@ -176,18 +178,15 @@ void cleanup(const int ret)
 	// Empty API port file, port 0 = truncate file
 	saveport(0);
 
-	// Close gravity database connection
-	gravityDB_close();
+	//Remove PID file
+	removepid();
 
 	// Remove shared memory objects
 	// Important: This invalidated all objects such as
-	//            counters-> ... Do this last when
-	//            terminating in main.c !
+	//            counters-> ... etc.
+	// This should be the last action when cleaning up
 	destroy_shmem();
 
-	//Remove PID file
-	removepid();
-
 	char buffer[42] = { 0 };
 	format_time(buffer, 0, timer_elapsed_msec(EXIT_TIMER));
 	logg("########## FTL terminated after%s (code %i)! ##########", buffer, ret);

From 62d20a97a7f13559680a2fec0f696f034545a9f7 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 12:45:44 +0100
Subject: [PATCH 4/8] Improve process-already-running detection

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/main.c               |  1 +
 src/procps.c             | 46 +++++++++++++++++++++++++++++++---------
 src/procps.h             |  2 +-
 src/shmem.c              | 23 ++++++++++----------
 src/syscalls/vsnprintf.c |  7 ------
 5 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/src/main.c b/src/main.c
index fe4de7c9f..74d391c44 100644
--- a/src/main.c
+++ b/src/main.c
@@ -59,6 +59,7 @@ int main (int argc, char* argv[])
 	if(!init_shmem(true))
 	{
 		logg("Initialization of shared memory failed.");
+		// Check if there is already a running FTL process
 		check_running_FTL();
 		return EXIT_FAILURE;
 	}
diff --git a/src/procps.c b/src/procps.c
index c3c16f1d7..ebd05e71b 100644
--- a/src/procps.c
+++ b/src/procps.c
@@ -44,7 +44,7 @@ static bool get_process_name(const pid_t pid, char name[128])
 static bool get_process_ppid(const pid_t pid, pid_t *ppid)
 {
 	// Try to open status file
-	char filename[sizeof("/proc/%u/task/%u/comm") + sizeof(int)*3 * 2];
+	char filename[sizeof("/proc/%u/task/%u/status") + sizeof(int)*3 * 2];
 	snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
 	FILE *f = fopen(filename, "r");
 	if(f == NULL)
@@ -75,22 +75,25 @@ static bool get_process_creation_time(const pid_t pid, char timestr[84])
 	return true;
 }
 
-void check_running_FTL(void)
+bool check_running_FTL(void)
 {
-	//pid_t pid;
 	DIR *dirPos;
 	struct dirent *entry;
+	const pid_t ourselves = getpid();
+	bool process_running = false;
 
 	// Open /proc
 	errno = 0;
 	if ((dirPos = opendir("/proc")) == NULL)
 	{
-		logg("Dailed to access /proc: %s", strerror(errno));
-		return;
+		logg("Failed to access /proc: %s", strerror(errno));
+		return false;
 	}
 
 	// Loop over entries in /proc
 	// This is much more efficient than iterating over all possible PIDs
+	pid_t last_pid = 0;
+	size_t last_len = 0u;
 	while ((entry = readdir(dirPos)) != NULL)
 	{
 		// We are only interested in subdirectories of /proc
@@ -104,7 +107,7 @@ void check_running_FTL(void)
 		const pid_t pid = strtol(entry->d_name, NULL, 10);
 
 		// Skip our own process
-		if(pid == getpid())
+		if(pid == ourselves)
 			continue;
 
 		// Get process name
@@ -112,6 +115,10 @@ void check_running_FTL(void)
 		if(!get_process_name(pid, name))
 			continue;
 
+		// Only process this is this is our own process
+		if(strcasecmp(name, PROCESS_NAME) != 0)
+			continue;
+
 		// Get parent process ID (PPID)
 		pid_t ppid;
 		if(!get_process_ppid(pid, &ppid))
@@ -123,11 +130,30 @@ void check_running_FTL(void)
 		char timestr[84] = { 0 };
 		get_process_creation_time(pid, timestr);
 
-		// Log this process if it is a duplicate of us
-		if(strcasecmp(name, PROCESS_NAME) == 0)
-			logg("---> %s is already running as PID %d (started %s, child of PID %i (%s))",
-			     PROCESS_NAME, pid, timestr, ppid, ppid_name);
+		// If this is the first process we log, add a header
+		if(!process_running)
+		{
+			process_running = true;
+			logg("HINT: %s is already running!", PROCESS_NAME);
+		}
+
+		if(last_pid != ppid)
+		{
+			// Independent process, may be child of init/systemd
+			logg("%s (%d) ──> %s (PID %d, started %s)",
+			     ppid_name, ppid, name, pid, timestr);
+			last_pid = pid;
+			last_len = snprintf(NULL, 0, "%s (%d) ──> ", ppid_name, ppid);
+		}
+		else
+		{
+			// Process parented by the one we analyzed before,
+			// highlight their relationship
+			logg("%*s └─> %s (PID %d, started %s)",
+			     (int)last_len, "", name, pid, timestr);
+		}
 	}
 
 	closedir(dirPos);
+	return process_running;
 }
diff --git a/src/procps.h b/src/procps.h
index 2993fbca5..f8522d5a2 100644
--- a/src/procps.h
+++ b/src/procps.h
@@ -10,6 +10,6 @@
 
 #ifndef PROCPS_H
 #define PROCPS_H
-void check_running_FTL(void);
+bool check_running_FTL(void);
 
 #endif // POCPS_H
\ No newline at end of file
diff --git a/src/shmem.c b/src/shmem.c
index 14d9382ba..5e4289a6c 100644
--- a/src/shmem.c
+++ b/src/shmem.c
@@ -211,7 +211,7 @@ bool strcmp_escaped(const char *a, const char *b)
 		free(aa);
 	if(Nb > 0)
 		free(bb);
-	
+
 	return result;
 }
 
@@ -426,7 +426,6 @@ bool init_shmem(bool create_new)
 			return false;
 		}
 	}
-	
 
 	/****************************** shared strings buffer ******************************/
 	// Try to create shared memory object
@@ -514,7 +513,8 @@ bool init_shmem(bool create_new)
 
 void destroy_shmem(void)
 {
-	pthread_mutex_destroy(&shmLock->lock);
+	if(shmLock != NULL)
+		pthread_mutex_destroy(&shmLock->lock);
 	shmLock = NULL;
 
 	delete_shm(&shm_lock);
@@ -742,15 +742,16 @@ bool realloc_shm(SharedMemory *sharedMemory, const size_t size1, const size_t si
 
 void delete_shm(SharedMemory *sharedMemory)
 {
-	// Unmap shared memory
-	int ret = munmap(sharedMemory->ptr, sharedMemory->size);
-	if(ret != 0)
-		logg("delete_shm(): munmap(%p, %zu) failed: %s", sharedMemory->ptr, sharedMemory->size, strerror(errno));
+	// Unmap shared memory (if mmapped)
+	if(sharedMemory->ptr != NULL)
+	{
+		if(munmap(sharedMemory->ptr, sharedMemory->size) != 0)
+			logg("delete_shm(): munmap(%p, %zu) failed: %s", sharedMemory->ptr, sharedMemory->size, strerror(errno));
+	}
 
-	// Now you can no longer `shm_open` the memory,
-	// and once all others unlink, it will be destroyed.
-	ret = shm_unlink(sharedMemory->name);
-	if(ret != 0)
+	// Now you can no longer `shm_open` the memory, and once all others
+	// unlink, it will be destroyed.
+	if(shm_unlink(sharedMemory->name) != 0)
 		logg("delete_shm(): shm_unlink(%s) failed: %s", sharedMemory->name, strerror(errno));
 }
 
diff --git a/src/syscalls/vsnprintf.c b/src/syscalls/vsnprintf.c
index 10f7b6c2c..2e1b5f088 100644
--- a/src/syscalls/vsnprintf.c
+++ b/src/syscalls/vsnprintf.c
@@ -15,13 +15,6 @@
 #undef vsnprintf
 int FTLvsnprintf(const char *file, const char *func, const int line, char *__restrict__ buffer, const size_t maxlen, const char *format, va_list args)
 {
-	// Sanity check
-	if(buffer == NULL)
-	{
-		syscalls_report_error("vsnprintf() called with NULL buffer",
-		                      stdout, 0, format, func, file, line);
-		return 0;
-	}
 	// Print into dynamically allocated memory
 	int _errno, length = 0;
 	do

From a6653e9952c39db0661338b22d8314e0b3df2027 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 12:56:15 +0100
Subject: [PATCH 5/8] Tests: Update tests for new expected output on two
 concurrent instances

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 test/test_suite.bats | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_suite.bats b/test/test_suite.bats
index bc6bb3b32..3db429dbc 100644
--- a/test/test_suite.bats
+++ b/test/test_suite.bats
@@ -15,7 +15,7 @@
   run bash -c 'su pihole -s /bin/sh -c "/home/pihole/pihole-FTL -f"'
   printf "%s\n" "${lines[@]}"
   [[ ${lines[9]} == *"Initialization of shared memory failed." ]]
-  [[ ${lines[10]} == *"--> pihole-FTL is already running as PID "* ]]
+  [[ ${lines[10]} == *"HINT: pihole-FTL is already running!"* ]]
 }
 
 @test "Starting tests without prior history" {

From 8244043ca194ae85f13a834e6249f53459a16f0e Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 13:02:18 +0100
Subject: [PATCH 6/8] Terminate threads before closing database connections and
 finishing shared memory

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/daemon.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/daemon.c b/src/daemon.c
index 93cd73c33..71efeb12e 100644
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -168,6 +168,20 @@ pid_t FTL_gettid(void)
 // Clean up on exit
 void cleanup(const int ret)
 {
+	// Terminate threads before closing database connections and finishing shared memory
+	if(telnet_listenthreadv4 != 0u)
+		pthread_cancel(telnet_listenthreadv4);
+	if(telnet_listenthreadv6 != 0u)
+		pthread_cancel(telnet_listenthreadv6);
+	if(socket_listenthread != 0u)
+		pthread_cancel(socket_listenthread);
+	if(DBthread != 0u)
+		pthread_cancel(DBthread);
+	if(GCthread != 0u)
+		pthread_cancel(GCthread);
+	if(DNSclientthread != 0u)
+		pthread_cancel(DNSclientthread);
+
 	// Close gravity database connection
 	gravityDB_close();
 

From 70ee90ebfcd59aa21ecede11fa26ecb43d9835d2 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 13:44:31 +0100
Subject: [PATCH 7/8] Clean up after dnsmasq errors (port not available config
 errors, etc.)

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/daemon.c                 | 18 ++++++------------
 src/database/message-table.c |  6 ++++++
 src/dnsmasq_interface.c      | 12 ++++++------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/daemon.c b/src/daemon.c
index 71efeb12e..9433c7e34 100644
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -169,18 +169,12 @@ pid_t FTL_gettid(void)
 void cleanup(const int ret)
 {
 	// Terminate threads before closing database connections and finishing shared memory
-	if(telnet_listenthreadv4 != 0u)
-		pthread_cancel(telnet_listenthreadv4);
-	if(telnet_listenthreadv6 != 0u)
-		pthread_cancel(telnet_listenthreadv6);
-	if(socket_listenthread != 0u)
-		pthread_cancel(socket_listenthread);
-	if(DBthread != 0u)
-		pthread_cancel(DBthread);
-	if(GCthread != 0u)
-		pthread_cancel(GCthread);
-	if(DNSclientthread != 0u)
-		pthread_cancel(DNSclientthread);
+	pthread_cancel(telnet_listenthreadv4);
+	pthread_cancel(telnet_listenthreadv6);
+	pthread_cancel(socket_listenthread);
+	pthread_cancel(DBthread);
+	pthread_cancel(GCthread);
+	pthread_cancel(DNSclientthread);
 
 	// Close gravity database connection
 	gravityDB_close();
diff --git a/src/database/message-table.c b/src/database/message-table.c
index 936813d75..097462a2d 100644
--- a/src/database/message-table.c
+++ b/src/database/message-table.c
@@ -17,6 +17,8 @@
 #include "gravity-db.h"
 // cli_mode
 #include "../args.h"
+// cleanup()
+#include "../daemon.h"
 
 static const char *message_types[MAX_MESSAGE] =
 	{ "REGEX", "SUBNET", "HOSTNAME", "DNSMASQ_CONFIG" };
@@ -271,4 +273,8 @@ void logg_fatal_dnsmasq_message(const char *message)
 	// Log to database (we have to open the database at this point)
 	dbopen();
 	add_message(DNSMASQ_CONFIG_MESSAGE, message, 0);
+
+	// FTL will dies after this point, so we should make sure to clean up
+	// behind ourselves
+	cleanup(EXIT_FAILURE);
 }
diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c
index 54151abca..d5906a3d3 100644
--- a/src/dnsmasq_interface.c
+++ b/src/dnsmasq_interface.c
@@ -1740,12 +1740,12 @@ static void save_reply_type(const unsigned int flags, const union all_addr *addr
 	                            query->response;
 }
 
-pthread_t telnet_listenthreadv4 = 0u;
-pthread_t telnet_listenthreadv6 = 0u;
-pthread_t socket_listenthread = 0u;
-pthread_t DBthread = 0u;
-pthread_t GCthread = 0u;
-pthread_t DNSclientthread = 0u;
+pthread_t telnet_listenthreadv4;
+pthread_t telnet_listenthreadv6;
+pthread_t socket_listenthread;
+pthread_t DBthread;
+pthread_t GCthread;
+pthread_t DNSclientthread;
 
 void FTL_fork_and_bind_sockets(struct passwd *ent_pw)
 {

From cf9018501d3584b2b21f4c9fd090b4f6182af1a9 Mon Sep 17 00:00:00 2001
From: DL6ER <dl6er@dl6er.de>
Date: Wed, 17 Feb 2021 20:53:34 +0100
Subject: [PATCH 8/8] Do not detach threads we want to be able to cancel and
 add logfile log to shared memory locks. Other forks may want to log as well.

Signed-off-by: DL6ER <dl6er@dl6er.de>
---
 src/api/socket.c               |  14 ++---
 src/database/database-thread.c |   3 +-
 src/database/query-table.c     |  18 ++++--
 src/database/query-table.h     |   2 +-
 src/dnsmasq/dnsmasq.c          |   6 ++
 src/dnsmasq_interface.c        |   4 --
 src/gc.c                       |   4 +-
 src/log.c                      |  15 +----
 src/main.c                     |   7 ++-
 src/resolve.c                  |   3 +-
 src/shmem.c                    | 103 +++++++++++++++++++++++----------
 src/shmem.h                    |   6 ++
 12 files changed, 120 insertions(+), 65 deletions(-)

diff --git a/src/api/socket.c b/src/api/socket.c
index 438dcf908..00f1a15d8 100644
--- a/src/api/socket.c
+++ b/src/api/socket.c
@@ -421,8 +421,8 @@ void *telnet_listening_thread_IPv4(void *args)
 	if(!ipv4telnet)
 		return NULL;
 
-	// Listen as long as FTL is not killed
-	while(!killed)
+	// Listen as long as this thread is not canceled
+	while(true)
 	{
 		// Look for new clients that want to connect
 		const int csck = listener(telnetfd4, 4);
@@ -470,8 +470,8 @@ void *telnet_listening_thread_IPv6(void *args)
 	if(!ipv6telnet)
 		return NULL;
 
-	// Listen as long as FTL is not killed
-	while(!killed)
+	// Listen as long as this thread is not canceled
+	while(true)
 	{
 		// Look for new clients that want to connect
 		const int csck = listener(telnetfd6, 6);
@@ -519,8 +519,8 @@ void *socket_listening_thread(void *args)
 		return NULL;
 	}
 
-	// Listen as long as FTL is not killed
-	while(!killed)
+	// Listen as long as this thread is not canceled
+	while(true)
 	{
 		// Look for new clients that want to connect
 		const int csck = listener(socketfd, 0);
@@ -541,7 +541,7 @@ void *socket_listening_thread(void *args)
 			logg("WARNING: Unable to open socket processing thread: %s", strerror(errno));
 		}
 	}
-	return false;
+	return NULL;
 }
 
 bool ipv6_available(void)
diff --git a/src/database/database-thread.c b/src/database/database-thread.c
index bfb84509a..6a26f74ac 100644
--- a/src/database/database-thread.c
+++ b/src/database/database-thread.c
@@ -36,7 +36,8 @@ void *DB_thread(void *val)
 	// to the database
 	time_t lastDBsave = time(NULL) - time(NULL)%config.DBinterval;
 
-	while(!killed)
+	// Run as long as this thread is not canceled
+	while(true)
 	{
 		if(FTL_DB_avail())
 		{
diff --git a/src/database/query-table.c b/src/database/query-table.c
index 2e3af7e21..677e887e5 100644
--- a/src/database/query-table.c
+++ b/src/database/query-table.c
@@ -42,8 +42,14 @@ int get_number_of_queries_in_DB(void)
 	return result;
 }
 
-void DB_save_queries(void)
+bool DB_save_queries(void)
 {
+	// The database may be unavailable, e.g. when disabled
+	if(!FTL_DB_avail())
+	{
+		return false;
+	}
+
 	// Start database timer
 	if(config.debug & DEBUG_DATABASE)
 		timer_start(DATABASE_WRITE_TIMER);
@@ -65,7 +71,7 @@ void DB_save_queries(void)
 		}
 
 		logg("%s: Storing queries in long-term database failed: %s", text, sqlite3_errstr(rc));
-		return;
+		return false;
 	}
 
 	rc = sqlite3_prepare_v2(FTL_db, "INSERT INTO queries VALUES (NULL,?,?,?,?,?,?,?)", -1, &stmt, NULL);
@@ -88,7 +94,7 @@ void DB_save_queries(void)
 		logg("%s: Storing queries in long-term database failed: %s\n", text, sqlite3_errstr(rc));
 		logg("%s  Keeping queries in memory for later new attempt", spaces);
 		saving_failed_before = true;
-		return;
+		return false;
 	}
 
 	// Get last ID stored in the database
@@ -230,7 +236,7 @@ void DB_save_queries(void)
 		else
 			dbclose();
 
-		return;
+		return false;
 	}
 
 	// Finish prepared statement
@@ -247,7 +253,7 @@ void DB_save_queries(void)
 		else
 			dbclose();
 
-		return;
+		return false;
 	}
 
 	// Store index for next loop interation round and update last time stamp
@@ -268,6 +274,8 @@ void DB_save_queries(void)
 			saving_failed_before = false;
 		}
 	}
+
+	return true;
 }
 
 void delete_old_queries_in_DB(void)
diff --git a/src/database/query-table.h b/src/database/query-table.h
index 2846d850b..d74d2257b 100644
--- a/src/database/query-table.h
+++ b/src/database/query-table.h
@@ -12,7 +12,7 @@
 
 int get_number_of_queries_in_DB(void);
 void delete_old_queries_in_DB(void);
-void DB_save_queries(void);
+bool DB_save_queries(void);
 void DB_read_queries(void);
 
 #endif //DATABASE_QUERY_TABLE_H
diff --git a/src/dnsmasq/dnsmasq.c b/src/dnsmasq/dnsmasq.c
index e76851c23..9970bcb3c 100644
--- a/src/dnsmasq/dnsmasq.c
+++ b/src/dnsmasq/dnsmasq.c
@@ -19,6 +19,8 @@
 
 #include "dnsmasq.h"
 #include "../dnsmasq_interface.h"
+// killed
+#include "../signals.h"
 
 struct daemon *daemon;
 
@@ -1026,6 +1028,10 @@ int main_dnsmasq (int argc, char **argv)
   /* Using inotify, have to select a resolv file at startup */
   poll_resolv(1, 0, now);
 #endif
+
+  /*** Pi-hole modification ***/
+  terminate = killed;
+  /****************************/
   
   while (!terminate)
     {
diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c
index d5906a3d3..96c2ba2fa 100644
--- a/src/dnsmasq_interface.c
+++ b/src/dnsmasq_interface.c
@@ -1768,10 +1768,6 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw)
 	pthread_attr_t attr;
 	// Initialize thread attributes object with default attribute values
 	pthread_attr_init(&attr);
-	// When a detached thread terminates, its resources are automatically
-	// released back to the system without the need for another thread to
-	// join with the terminated thread
-	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 
 	// Start TELNET IPv4 thread
 	if(pthread_create( &telnet_listenthreadv4, &attr, telnet_listening_thread_IPv4, NULL ) != 0)
diff --git a/src/gc.c b/src/gc.c
index 700ef51e8..a76287916 100644
--- a/src/gc.c
+++ b/src/gc.c
@@ -41,7 +41,9 @@ void *GC_thread(void *val)
 	// Remember when we last ran the actions
 	time_t lastGCrun = time(NULL) - time(NULL)%GCinterval;
 	time_t lastRateLimitCleaner = time(NULL);
-	while(!killed)
+
+	// Run as long as this thread is not canceled
+	while(true)
 	{
 		const time_t now = time(NULL);
 		if((unsigned int)(now - lastRateLimitCleaner) >= config.rate_limit.interval)
diff --git a/src/log.c b/src/log.c
index 648bb2975..0c5c2b79a 100644
--- a/src/log.c
+++ b/src/log.c
@@ -18,14 +18,13 @@
 #include "main.h"
 // global variable daemonmode
 #include "args.h"
-// global counters variable
+// global counters variable and shared logfile lock
 #include "shmem.h"
 // main_pid()
 #include "signals.h"
 // logg_fatal_dnsmasq_message()
 #include "database/message-table.h"
 
-static pthread_mutex_t lock;
 static FILE *logfile = NULL;
 static bool FTL_log_ready = false;
 static bool print_log = true, print_stdout = true;
@@ -46,14 +45,6 @@ void open_FTL_log(const bool init)
 {
 	if(init)
 	{
-		// Initialize logging mutex
-		if (pthread_mutex_init(&lock, NULL) != 0)
-		{
-			printf("FATAL: Log mutex init failed\n");
-			// Return failure
-			exit(EXIT_FAILURE);
-		}
-
 		// Obtain log file location
 		getLogFilePath();
 	}
@@ -111,7 +102,7 @@ void _FTL_log(const bool newline, const char *format, ...)
 	if(!print_log && !print_stdout)
 		return;
 
-	pthread_mutex_lock(&lock);
+	lock_log();
 
 	get_timestr(timestring, time(NULL), true);
 
@@ -175,7 +166,7 @@ void _FTL_log(const bool newline, const char *format, ...)
 		close_FTL_log();
 	}
 
-	pthread_mutex_unlock(&lock);
+	unlock_log();
 }
 
 // Log helper activity (may be script or lua)
diff --git a/src/main.c b/src/main.c
index 74d391c44..85b28e63f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -97,12 +97,15 @@ int main (int argc, char* argv[])
 	main_dnsmasq(argc_dnsmasq, argv_dnsmasq);
 
 	logg("Shutting down...");
+	// Extra grace time is needed as dnsmasq script-helpers may not be
+	// terminating immediately
+	sleepms(250);
 
 	// Save new queries to database (if database is used)
 	if(config.DBexport)
 	{
-		DB_save_queries();
-		logg("Finished final database update");
+		if(DB_save_queries())
+			logg("Finished final database update");
 	}
 
 	cleanup(exit_code);
diff --git a/src/resolve.c b/src/resolve.c
index 0365c5931..0a4aa9287 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -584,7 +584,8 @@ void *DNSclient_thread(void *val)
 	// Initial delay until we first try to resolve anything
 	sleepms(2000);
 
-	while(!killed)
+	// Run as long as this thread is not canceled
+	while(true)
 	{
 		// Run whenever necessary to resolve only new clients and
 		// upstream servers
diff --git a/src/shmem.c b/src/shmem.c
index 5e4289a6c..0b13884f5 100644
--- a/src/shmem.c
+++ b/src/shmem.c
@@ -21,7 +21,7 @@
 #include "regex_r.h"
 
 /// The version of shared memory used
-#define SHARED_MEMORY_VERSION 11
+#define SHARED_MEMORY_VERSION 12
 
 /// The name of the shared memory. Use this when connecting to the shared memory.
 #define SHMEM_PATH "/dev/shm"
@@ -68,7 +68,11 @@ typedef struct {
 	pthread_mutex_t lock;
 	bool waitingForLock;
 } ShmLock;
-static ShmLock *shmLock = NULL;
+typedef struct {
+	ShmLock shmem;
+	ShmLock logfile;
+} ShmLocks;
+static ShmLocks *shmLock = NULL;
 static ShmSettings *shmSettings = NULL;
 
 static int pagesize;
@@ -337,17 +341,37 @@ static void remap_shm(void)
 	local_shm_counter = shmSettings->global_shm_counter;
 }
 
-void _lock_shm(const char* func, const int line, const char * file) {
-	// Signal that FTL is waiting for a lock
-	shmLock->waitingForLock = true;
+static void _lock(ShmLock *lock, const bool is_shmem, const char* func, const int line, const char * file)
+{
+	// Signal that FTL is waiting for the lock
+	lock->waitingForLock = true;
+
+	if(config.debug & DEBUG_LOCKS && is_shmem)
+		logg("Waiting for SHM lock in %s() (%s:%i)", func, file, line);
 
-	if(config.debug & DEBUG_LOCKS)
-		logg("Waiting for lock in %s() (%s:%i)", func, file, line);
+	int result = pthread_mutex_lock(&lock->lock);
 
-	int result = pthread_mutex_lock(&shmLock->lock);
+	if(config.debug & DEBUG_LOCKS && is_shmem)
+		logg("Obtained lock SHM for %s() (%s:%i)", func, file, line);
+
+	// Turn off the waiting for lock signal to notify everyone who was
+	// deferring to FTL that they can jump in the lock queue.
+	lock->waitingForLock = false;
 
-	if(config.debug & DEBUG_LOCKS)
-		logg("Obtained lock for %s() (%s:%i)", func, file, line);
+	if(result == EOWNERDEAD) {
+		// Try to make the lock consistent if the other process died while
+		// holding the lock
+		result = pthread_mutex_consistent(&lock->lock);
+	}
+
+	if(result != 0 && is_shmem)
+		logg("Failed to obtain SHM lock: %s", strerror(result));
+}
+
+// Obtain SHMEM lock
+void _lock_shm(const char* func, const int line, const char * file)
+{
+	_lock(&shmLock->shmem, true, func, line, file);
 
 	// Check if this process needs to remap the shared memory objects
 	if(shmSettings != NULL &&
@@ -358,31 +382,43 @@ void _lock_shm(const char* func, const int line, const char * file) {
 		             local_shm_counter, shmSettings->global_shm_counter);
 		remap_shm();
 	}
+}
 
-	// Turn off the waiting for lock signal to notify everyone who was
-	// deferring to FTL that they can jump in the lock queue.
-	shmLock->waitingForLock = false;
-
-	if(result == EOWNERDEAD) {
-		// Try to make the lock consistent if the other process died while
-		// holding the lock
-		result = pthread_mutex_consistent(&shmLock->lock);
-	}
-
-	if(result != 0)
-		logg("Failed to obtain SHM lock: %s", strerror(result));
+// Obtain log file lock
+void _lock_log(const char* func, const int line, const char * file)
+{
+	// Locks may have not been initialized so far
+	if(shmLock == NULL)
+		return;
+	_lock(&shmLock->logfile, false, func, line, file);
 }
 
-void _unlock_shm(const char* func, const int line, const char * file) {
-	int result = pthread_mutex_unlock(&shmLock->lock);
+static void _unlock(ShmLock *lock, const bool is_shmem, const char* func, const int line, const char * file)
+{
+	int result = pthread_mutex_unlock(&lock->lock);
 
-	if(config.debug & DEBUG_LOCKS)
+	if(config.debug & DEBUG_LOCKS && is_shmem)
 		logg("Removed lock in %s() (%s:%i)", func, file, line);
 
-	if(result != 0)
+	if(result != 0 && is_shmem)
 		logg("Failed to unlock SHM lock: %s", strerror(result));
 }
 
+// Release SHM lock
+void _unlock_shm(const char* func, const int line, const char * file)
+{
+	_unlock(&shmLock->shmem, true, func, line, file);
+}
+
+// Release log file lock
+void _unlock_log(const char* func, const int line, const char * file)
+{
+	// Locks may have not been initialized so far
+	if(shmLock == NULL)
+		return;
+	_unlock(&shmLock->logfile, false, func, line, file);
+}
+
 bool init_shmem(bool create_new)
 {
 	// Get kernel's page size
@@ -390,14 +426,16 @@ bool init_shmem(bool create_new)
 
 	/****************************** shared memory lock ******************************/
 	// Try to create shared memory object
-	shm_lock = create_shm(SHARED_LOCK_NAME, sizeof(ShmLock), create_new);
+	shm_lock = create_shm(SHARED_LOCK_NAME, sizeof(ShmLocks), create_new);
 	if(shm_lock.ptr == NULL)
 		return false;
-	shmLock = (ShmLock*) shm_lock.ptr;
+	shmLock = (ShmLocks*) shm_lock.ptr;
 	if(create_new)
 	{
-		shmLock->lock = create_mutex();
-		shmLock->waitingForLock = false;
+		shmLock->shmem.lock = create_mutex();
+		shmLock->shmem.waitingForLock = false;
+		shmLock->logfile.lock = create_mutex();
+		shmLock->logfile.waitingForLock = false;
 	}
 
 	/****************************** shared counters struct ******************************/
@@ -514,7 +552,10 @@ bool init_shmem(bool create_new)
 void destroy_shmem(void)
 {
 	if(shmLock != NULL)
-		pthread_mutex_destroy(&shmLock->lock);
+	{
+		pthread_mutex_destroy(&shmLock->shmem.lock);
+		pthread_mutex_destroy(&shmLock->logfile.lock);
+	}
 	shmLock = NULL;
 
 	delete_shm(&shm_lock);
diff --git a/src/shmem.h b/src/shmem.h
index 601cb9cea..619fdd135 100644
--- a/src/shmem.h
+++ b/src/shmem.h
@@ -89,10 +89,16 @@ void delete_shm(SharedMemory *sharedMemory);
 /// Block until a lock can be obtained
 #define lock_shm() _lock_shm(__FUNCTION__, __LINE__, __FILE__)
 void _lock_shm(const char* func, const int line, const char* file);
+#define lock_log() _lock_log(__FUNCTION__, __LINE__, __FILE__)
+void _lock_log(const char* func, const int line, const char* file);
 
 /// Unlock the lock. Only call this if there is an active lock.
 #define unlock_shm() _unlock_shm(__FUNCTION__, __LINE__, __FILE__)
 void _unlock_shm(const char* func, const int line, const char* file);
+#define unlock_log() _unlock_log(__FUNCTION__, __LINE__, __FILE__)
+void _unlock_log(const char* func, const int line, const char * file);
+
+/// Block until a lock can be obtained
 
 bool init_shmem(bool create_new);
 void destroy_shmem(void);