From a6ac4dd1176f7fc6f12e8513ec49da58607a6922 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel 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