[network-manager-openvpn: 2/6] service: fix handling child processes and their cleanup
- From: Thomas Haller <thaller src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [network-manager-openvpn: 2/6] service: fix handling child processes and their cleanup
- Date: Tue, 2 Feb 2016 16:59:30 +0000 (UTC)
commit 241562690640b02b4f3a338d39404385baca547f
Author: Thomas Haller <thaller redhat com>
Date: Fri Jan 29 17:07:25 2016 +0100
service: fix handling child processes and their cleanup
Previously, ensure_killed() would never be canceled, meaning
2 seconds after sending SIGTERM to the openvpn process, we would
send a SIGKILL signal (to whatever process happens to be around).
Also, the child-watch callback would sooner or later be invoked
when the process finally dies. But since we never unsubscribe
the watcher, we can get the signal for a previously died process.
Also, calling waitpid() is wrong inside a child-watch since the process
was already reaped.
Rework this and keep track of the pending processes in a list
@pids_pending_list. The currently active process is the one
with matching priv->pid.
On shutdown, we wait for all processes to terminate (and still sent
them the final SIGKILL as necessary).
src/nm-openvpn-service.c | 217 +++++++++++++++++++++++++++++++++++-----------
1 files changed, 165 insertions(+), 52 deletions(-)
---
diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index 704e01a..c654651 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -59,6 +59,10 @@
static gboolean debug = FALSE;
static GMainLoop *loop = NULL;
+static struct {
+ GSList *pids_pending_list;
+} gl/*obal*/;
+
#define NM_OPENVPN_HELPER_PATH LIBEXECDIR"/nm-openvpn-service-openvpn-helper"
G_DEFINE_TYPE (NMOpenvpnPlugin, nm_openvpn_plugin, NM_TYPE_VPN_SERVICE_PLUGIN)
@@ -94,6 +98,13 @@ typedef struct {
gboolean address;
} ValidProperty;
+typedef struct {
+ GPid pid;
+ guint watch_id;
+ guint kill_id;
+ NMOpenvpnPlugin *plugin;
+} PidsPendingData;
+
static ValidProperty valid_properties[] = {
{ NM_OPENVPN_KEY_AUTH, G_TYPE_STRING, 0, 0, FALSE },
{ NM_OPENVPN_KEY_CA, G_TYPE_STRING, 0, 0, FALSE },
@@ -147,6 +158,130 @@ static ValidProperty valid_secrets[] = {
{ NULL, G_TYPE_NONE, FALSE }
};
+/*****************************************************************************/
+
+static void
+pids_pending_data_free (PidsPendingData *pid_data)
+{
+ nm_clear_g_source (&pid_data->watch_id);
+ nm_clear_g_source (&pid_data->kill_id);
+ if (pid_data->plugin)
+ g_object_remove_weak_pointer ((GObject *) pid_data->plugin, (gpointer *) &pid_data->plugin);
+ g_slice_free (PidsPendingData, pid_data);
+}
+
+static PidsPendingData *
+pids_pending_get (GPid pid)
+{
+ GSList *iter;
+
+ for (iter = gl.pids_pending_list; iter; iter = iter->next) {
+ if (((PidsPendingData *) iter->data)->pid == pid)
+ return iter->data;
+ }
+ g_return_val_if_reached (NULL);
+}
+
+static void openvpn_child_terminated (NMOpenvpnPlugin *plugin, GPid pid, gint status);
+
+static void
+pids_pending_child_watch_cb (GPid pid, gint status, gpointer user_data)
+{
+ PidsPendingData *pid_data = user_data;
+ NMOpenvpnPlugin *plugin;
+
+ if (WIFEXITED (status)) {
+ int exit_status;
+
+ exit_status = WEXITSTATUS (status);
+ if (exit_status != 0)
+ g_warning ("openvpn[%ld] exited with error code %d", (long) pid, exit_status);
+ else
+ g_message ("openvpn[%ld] exited with success", (long) pid);
+ }
+ else if (WIFSTOPPED (status))
+ g_warning ("openvpn[%ld] stopped unexpectedly with signal %d", (long) pid, WSTOPSIG (status));
+ else if (WIFSIGNALED (status))
+ g_warning ("openvpn[%ld] died with signal %d", (long) pid, WTERMSIG (status));
+ else
+ g_warning ("openvpn[%ld] died from an unnatural cause", (long) pid);
+
+ g_return_if_fail (pid_data);
+ g_return_if_fail (pid_data->pid == pid);
+ g_return_if_fail (g_slist_find (gl.pids_pending_list, pid_data));
+
+ plugin = pid_data->plugin;
+
+ pid_data->watch_id = 0;
+ gl.pids_pending_list = g_slist_remove (gl.pids_pending_list , pid_data);
+ pids_pending_data_free (pid_data);
+
+ if (plugin)
+ openvpn_child_terminated (plugin, pid, status);
+}
+
+static void
+pids_pending_add (GPid pid, NMOpenvpnPlugin *plugin)
+{
+ PidsPendingData *pid_data;
+
+ g_return_if_fail (NM_IS_OPENVPN_PLUGIN (plugin));
+ g_return_if_fail (pid > 0);
+
+ g_message ("openvpn[%ld] started", (long) pid);
+
+ pid_data = g_slice_new (PidsPendingData);
+ pid_data->pid = pid;
+ pid_data->kill_id = 0;
+ pid_data->watch_id = g_child_watch_add (pid, pids_pending_child_watch_cb, pid_data);
+ pid_data->plugin = plugin;
+ g_object_add_weak_pointer ((GObject *) plugin, (gpointer *) &pid_data->plugin);
+
+ gl.pids_pending_list = g_slist_prepend (gl.pids_pending_list, pid_data);
+}
+
+static gboolean
+pids_pending_ensure_killed (gpointer user_data)
+{
+ PidsPendingData *pid_data = user_data;
+
+ g_return_val_if_fail (pid_data && pid_data == pids_pending_get (pid_data->pid), FALSE);
+
+ g_message ("openvpn[%ld]: send SIGKILL", (long) pid_data->pid);
+
+ pid_data->kill_id = 0;
+ kill (pid_data->pid, SIGKILL);
+ return FALSE;
+}
+
+static void
+pids_pending_send_sigterm (GPid pid)
+{
+ PidsPendingData *pid_data;
+
+ pid_data = pids_pending_get (pid);
+ g_return_if_fail (pid_data);
+
+ g_message ("openvpn[%ld]: send SIGTERM", (long) pid);
+
+ kill (pid, SIGTERM);
+ pid_data->kill_id = g_timeout_add (2000, pids_pending_ensure_killed, pid_data);
+}
+
+static void
+pids_pending_wait_for_processes (GMainLoop *main_loop)
+{
+ if (gl.pids_pending_list) {
+ g_message ("wait for %u openvpn processes to terminate...", g_slist_length
(gl.pids_pending_list));
+
+ do {
+ g_main_context_iteration (g_main_loop_get_context (main_loop), TRUE);
+ } while (gl.pids_pending_list);
+ }
+}
+
+/*****************************************************************************/
+
static gboolean
validate_address (const char *address)
{
@@ -483,7 +618,7 @@ handle_auth (NMOpenvpnPluginIOData *io_data,
}
static gboolean
-handle_management_socket (NMVpnServicePlugin *plugin,
+handle_management_socket (NMOpenvpnPlugin *plugin,
GIOChannel *source,
GIOCondition condition,
NMVpnPluginFailure *out_failure)
@@ -525,7 +660,7 @@ handle_management_socket (NMVpnServicePlugin *plugin,
g_message ("Requesting new secrets: '%s' (%s)", message,
joined);
g_free (joined);
}
- nm_vpn_service_plugin_secrets_required (plugin, message, (const char
**) hints);
+ nm_vpn_service_plugin_secrets_required ((NMVpnServicePlugin *)
plugin, message, (const char **) hints);
} else {
/* Interactive not allowed, can't ask for more secrets */
g_warning ("More secrets required but cannot ask interactively");
@@ -579,11 +714,11 @@ out:
static gboolean
nm_openvpn_socket_data_cb (GIOChannel *source, GIOCondition condition, gpointer user_data)
{
- NMVpnServicePlugin *plugin = NM_VPN_SERVICE_PLUGIN (user_data);
+ NMOpenvpnPlugin *plugin = NM_OPENVPN_PLUGIN (user_data);
NMVpnPluginFailure failure = NM_VPN_PLUGIN_FAILURE_CONNECT_FAILED;
if (!handle_management_socket (plugin, source, condition, &failure)) {
- nm_vpn_service_plugin_failure (plugin, failure);
+ nm_vpn_service_plugin_failure ((NMVpnServicePlugin *) plugin, failure);
return FALSE;
}
@@ -644,34 +779,26 @@ nm_openvpn_schedule_connect_timer (NMOpenvpnPlugin *plugin)
}
static void
-openvpn_watch_cb (GPid pid, gint status, gpointer user_data)
+openvpn_child_terminated (NMOpenvpnPlugin *plugin, GPid pid, gint status)
{
- NMVpnServicePlugin *plugin = NM_VPN_SERVICE_PLUGIN (user_data);
- NMOpenvpnPluginPrivate *priv = NM_OPENVPN_PLUGIN_GET_PRIVATE (plugin);
+ NMOpenvpnPluginPrivate *priv;
NMVpnPluginFailure failure = NM_VPN_PLUGIN_FAILURE_CONNECT_FAILED;
- guint error = 0;
gboolean good_exit = FALSE;
- if (WIFEXITED (status)) {
- error = WEXITSTATUS (status);
- if (error != 0)
- g_warning ("openvpn[%ld] exited with error code %d", (long) pid, error);
- else
- g_message ("openvpn[%ld] exited with success", (long) pid);
- }
- else if (WIFSTOPPED (status))
- g_warning ("openvpn[%ld] stopped unexpectedly with signal %d", (long) pid, WSTOPSIG (status));
- else if (WIFSIGNALED (status))
- g_warning ("openvpn[%ld] died with signal %d", (long) pid, WTERMSIG (status));
- else
- g_warning ("openvpn[%ld] died from an unknown cause", (long) pid);
+ g_return_if_fail (NM_IS_OPENVPN_PLUGIN (plugin));
+ priv = NM_OPENVPN_PLUGIN_GET_PRIVATE (plugin);
/* Reap child if needed. */
- waitpid (priv->pid, NULL, WNOHANG);
+ if (priv->pid != pid) {
+ /* the dead child is not the currently active process. Nothing to do, we just
+ * reaped the PID. */
+ return;
+ }
+
priv->pid = 0;
/* OpenVPN doesn't supply useful exit codes :( */
- if (error == 0)
+ if (WIFEXITED (status) && WEXITSTATUS (status) == 0)
good_exit = TRUE;
/* Try to get the last bits of data from openvpn */
@@ -688,9 +815,9 @@ openvpn_watch_cb (GPid pid, gint status, gpointer user_data)
}
if (good_exit)
- nm_vpn_service_plugin_disconnect (plugin, NULL);
+ nm_vpn_service_plugin_disconnect ((NMVpnServicePlugin *) plugin, NULL);
else
- nm_vpn_service_plugin_failure (plugin, failure);
+ nm_vpn_service_plugin_failure ((NMVpnServicePlugin *) plugin, failure);
}
static gboolean
@@ -933,7 +1060,6 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
NMOpenvpnPluginPrivate *priv = NM_OPENVPN_PLUGIN_GET_PRIVATE (plugin);
const char *openvpn_binary, *auth, *connection_type, *tmp, *tmp2, *tmp3, *tmp4;
GPtrArray *args;
- GSource *openvpn_watch;
GPid pid;
gboolean dev_type_is_tap;
char *stmp;
@@ -1451,13 +1577,10 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
}
free_openvpn_args (args);
- g_message ("openvpn started with pid %d", pid);
+ pids_pending_add (pid, plugin);
+ g_warn_if_fail (!priv->pid);
priv->pid = pid;
- openvpn_watch = g_child_watch_source_new (pid);
- g_source_set_callback (openvpn_watch, (GSourceFunc) openvpn_watch_cb, plugin, NULL);
- g_source_attach (openvpn_watch, NULL);
- g_source_unref (openvpn_watch);
/* Listen to the management socket for a few connection types:
PASSWORD: Will require username and password
@@ -1533,35 +1656,18 @@ check_need_secrets (NMSettingVpn *s_vpn, gboolean *need_secrets)
}
static gboolean
-ensure_killed (gpointer data)
-{
- int pid = GPOINTER_TO_INT (data);
-
- if (kill (pid, 0) == 0)
- kill (pid, SIGKILL);
-
- return FALSE;
-}
-
-static gboolean
real_disconnect (NMVpnServicePlugin *plugin,
GError **err)
{
NMOpenvpnPluginPrivate *priv = NM_OPENVPN_PLUGIN_GET_PRIVATE (plugin);
- if (priv->pid) {
- if (kill (priv->pid, SIGTERM) == 0)
- g_timeout_add (2000, ensure_killed, GINT_TO_POINTER (priv->pid));
- else
- kill (priv->pid, SIGKILL);
+ g_clear_pointer (&priv->mgt_path, g_free);
- g_message ("Terminated openvpn daemon with PID %d.", priv->pid);
+ if (priv->pid) {
+ pids_pending_send_sigterm (priv->pid);
priv->pid = 0;
}
- g_free (priv->mgt_path);
- priv->mgt_path = NULL;
-
return TRUE;
}
@@ -1737,6 +1843,11 @@ dispose (GObject *object)
nm_clear_g_source (&priv->connect_timer);
+ if (priv->pid) {
+ pids_pending_send_sigterm (priv->pid);
+ priv->pid = 0;
+ }
+
G_OBJECT_CLASS (nm_openvpn_plugin_parent_class)->dispose (object);
}
@@ -1894,9 +2005,11 @@ main (int argc, char *argv[])
setup_signals ();
g_main_loop_run (loop);
+ g_object_unref (plugin);
+
+ pids_pending_wait_for_processes (loop);
g_main_loop_unref (loop);
- g_object_unref (plugin);
exit (EXIT_SUCCESS);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]