dnsmasq: kill running process from pidfile synchronously

This should avoid a race when stopping dnsmasq.

This is still a hack, because we don't want to ever wait
for a process to terminate.

A better solution would be to have nm-dnsmasq-manager managing
the global state. Not only the dnsmasq instance of on NMDevice.
Instead it should keep track of all the child processes it starts
and it wants to start. This way, starting a new process should be
delayed until any (child or non-child) instances are gone.
That however would be a bigger change.

https://bugzilla.gnome.org/show_bug.cgi?id=728342
https://bugzilla.gnome.org/show_bug.cgi?id=762008

1 files changed, 22 insertions, 15 deletions

diff --git a/src/dnsmasq-manager/nm-dnsmasq-manager.c b/src/dnsmasq-manager/nm-dnsmasq-manager.c
index f4a6208d05..6ebbcf5fb5 100644
--- a/src/dnsmasq-manager/nm-dnsmasq-manager.c
+++ b/src/dnsmasq-manager/nm-dnsmasq-manager.c
@@ -33,6 +33,7 @@
 #include "nm-dnsmasq-utils.h"
 #include "nm-utils.h"
 #include "NetworkManagerUtils.h"
+#include "nm-core-internal.h"
 
 typedef struct {
 	char *iface;
@@ -302,35 +303,41 @@ create_dm_cmd_line (const char *iface,
 }
 
 static void
-kill_existing_for_iface (const char *iface, const char *pidfile)
+kill_existing_by_pidfile (const char *pidfile)
 {
 	char *contents = NULL;
-	glong pid;
-	char *proc_path = NULL;
+	pid_t pid;
+	char proc_path[250];
 	char *cmdline_contents = NULL;
+	guint64 start_time;
+	const char *exe;
 
-	if (!g_file_get_contents (pidfile, &contents, NULL, NULL))
+	if (   !pidfile
+	    || !g_file_get_contents (pidfile, &contents, NULL, NULL))
+		return;
+
+	pid = _nm_utils_ascii_str_to_int64 (contents, 10, 1, G_MAXUINT64, 0);
+	if (pid == 0)
 		goto out;
 
-	pid = strtol (contents, NULL, 10);
-	if (pid < 1 || pid > INT_MAX)
+	start_time = nm_utils_get_start_time_for_pid (pid, NULL, NULL);
+	if (start_time == 0)
 		goto out;
 
-	proc_path = g_strdup_printf ("/proc/%ld/cmdline", pid);
+	nm_sprintf_buf (proc_path, "/proc/%lld/cmdline", (long long) pid);
 	if (!g_file_get_contents (proc_path, &cmdline_contents, NULL, NULL))
 		goto out;
 
-	if (strstr (cmdline_contents, "bin/dnsmasq")) {
-		if (kill (pid, 0) == 0) {
-			nm_log_dbg (LOGD_SHARING, "Killing stale dnsmasq process %ld", pid);
-			kill (pid, SIGKILL);
-		}
-		unlink (pidfile);
+	exe = strrchr (cmdline_contents, '/');
+	if (   (exe && strcmp (&exe[1], "dnsmasq") == 0)
+	    || (strcmp (cmdline_contents, DNSMASQ_PATH) == 0)) {
+		nm_utils_kill_process_sync (pid, start_time, SIGKILL, LOGD_SHARING,
+		                            "dnsmasq", 0, 0, 500);
 	}
 
 out:
+	unlink (pidfile);
 	g_free (cmdline_contents);
-	g_free (proc_path);
 	g_free (contents);
 }
 
@@ -349,7 +356,7 @@ nm_dnsmasq_manager_start (NMDnsMasqManager *manager,
 
 	priv = NM_DNSMASQ_MANAGER_GET_PRIVATE (manager);
 
-	kill_existing_for_iface (priv->iface, priv->pidfile);
+	kill_existing_by_pidfile (priv->pidfile);
 
 	dm_cmd = create_dm_cmd_line (priv->iface, ip4_config, priv->pidfile, error);
 	if (!dm_cmd)