From a6ac4dd1176f7fc6f12e8513ec49da58607a6922 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 6 May 2015 11:52:27 +0200
Subject: [PATCH] dns: kill plugin child synchronously to avoid restart race
 (rh #1161232) (bgo #728342)

NM was killing the dnsmasq local caching nameserver process and immediately
starting a new one, and new process couldn't bind to 127.0.0.1 because the
old one hadn't quit yet.  Thus the new process quit, and the user was
left with no split DNS at all.

While this does introduce more synchronous waiting into the connection
process, it's not that much time and NM will kill dnsmasq if it hasn't
quit after 1 second.  The longer-term fix is to use dnsmasq's D-Bus
interface to update DNS without respawning it.

https://bugzilla.gnome.org/show_bug.cgi?id=728342
https://bugzilla.redhat.com/show_bug.cgi?id=1161232

This is a rework of 10aff12526a2fc4b2d099df2710fdb040ccd9e4c. The newer
branches have ff3b753 (core: use nm_utils_kill_child_async() and
nm_utils_kill_child_sync()) which in turn relies on 1f84185 (core: add
nm_utils_kill_child_async() and nm_utils_kill_child_sync() function) that is
not entirely trivial to backport.
---
 src/dns-manager/nm-dns-plugin.c | 46 ++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/src/dns-manager/nm-dns-plugin.c b/src/dns-manager/nm-dns-plugin.c
index e85b2a0..4f86d63 100644
--- a/src/dns-manager/nm-dns-plugin.c
+++ b/src/dns-manager/nm-dns-plugin.c
@@ -196,29 +196,6 @@ nm_dns_plugin_child_spawn (NMDnsPlugin *self,
 	return priv->pid;
 }
 
-typedef struct {
-	int pid;
-	char *progname;
-} KillInfo;
-
-static gboolean
-ensure_killed (gpointer data)
-{
-	KillInfo *info = data;
-
-	if (kill (info->pid, 0) == 0)
-		kill (info->pid, SIGKILL);
-
-	/* ensure the child is reaped */
-	nm_log_dbg (LOGD_DNS, "waiting for %s pid %d to exit", info->progname, info->pid);
-	waitpid (info->pid, NULL, 0);
-	nm_log_dbg (LOGD_DNS, "dnsmasq pid %d cleaned up", info->pid);
-
-	g_free (info->progname);
-	g_free (info);
-	return FALSE;
-}
-
 gboolean nm_dns_plugin_child_kill (NMDnsPlugin *self)
 {
 	NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
@@ -229,21 +206,30 @@ gboolean nm_dns_plugin_child_kill (NMDnsPlugin *self)
 	}
 
 	if (priv->pid) {
-		KillInfo *info;
 
 		if (kill (priv->pid, SIGTERM) == 0) {
-			info = g_malloc0 (sizeof (KillInfo));
-			info->pid = priv->pid;
-			info->progname = g_strdup (priv->progname);
-			g_timeout_add_seconds (2, ensure_killed, info);
-		} else {
+			int counter = 20;
+
+			/* Wait up to 2 seconds synchronously. */
+			nm_log_dbg (LOGD_DNS, "waiting for %s pid %d to exit", priv->progname, priv->pid);
+			while (counter--) {
+				if (waitpid (priv->pid, NULL, WNOHANG))
+					goto killed;
+				g_usleep (100000);
+			}
+		}
+
+		if (kill (priv->pid, 0) == 0) {
+			/* Not dead yet. */
 			kill (priv->pid, SIGKILL);
 
 			/* ensure the child is reaped */
 			nm_log_dbg (LOGD_DNS, "waiting for %s pid %d to exit", priv->progname, priv->pid);
 			waitpid (priv->pid, NULL, 0);
-			nm_log_dbg (LOGD_DNS, "%s pid %d cleaned up", priv->progname, priv->pid);
 		}
+
+killed:
+		nm_log_dbg (LOGD_DNS, "%s pid %d cleaned up", priv->progname, priv->pid);
 		priv->pid = 0;
 		g_free (priv->progname);
 		priv->progname = NULL;
-- 
2.4.0