From 362c4fe18810bb3ff0e71aebe795692d9388f31b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jul 2014 13:25:57 +0200 Subject: [PATCH 1/4] nmtst: ensure call to nm_utils_get_monotonic_timestamp_s() in nmtst_init() Signed-off-by: Thomas Haller --- include/nm-test-utils.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index 83c4080fe0..25aad9d2bd 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -292,6 +292,11 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ atexit (nmtst_free); g_once_init_leave (&atexit_registered, 1); } + +#ifdef NETWORK_MANAGER_UTILS_H + /* ensure that monotonic timestamp is called (because it initially logs a line) */ + nm_utils_get_monotonic_timestamp_s (); +#endif } #ifdef NM_LOGGING_H From c469e81f960ccf4437ddb9029c9ce04028c4bcf2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jul 2014 10:40:47 +0200 Subject: [PATCH 2/4] core/test: move src/tests/test-general over to nm-test-utils.h (nmtst) Signed-off-by: Thomas Haller --- src/tests/Makefile.am | 1 + src/tests/test-general.c | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index ee5be17d7b..a44c2ee3bd 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -5,6 +5,7 @@ AM_CPPFLAGS = \ -I$(top_builddir)/libnm-util \ -I$(top_srcdir)/src/logging \ -I$(top_srcdir)/src/platform \ + -I$(top_srcdir)/src/logging \ -I$(top_srcdir)/src/dhcp-manager \ -I$(top_srcdir)/src \ -I$(top_builddir)/src \ diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 11c03f0dbe..723598e8e9 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -24,8 +24,9 @@ #include #include "NetworkManagerUtils.h" -#include "nm-utils.h" +#include "nm-logging.h" +#include "nm-test-utils.h" static void test_nm_utils_ascii_str_to_int64_check (const char *str, guint base, gint64 min, @@ -588,14 +589,12 @@ test_connection_no_match_ip4_addr (void) /*******************************************/ +NMTST_DEFINE (); + int main (int argc, char **argv) { - g_test_init (&argc, &argv, NULL); - -#if !GLIB_CHECK_VERSION (2, 35, 0) - g_type_init (); -#endif + nmtst_init_with_logging (&argc, &argv, NULL, "ALL"); g_test_add_func ("/general/nm_utils_ascii_str_to_int64", test_nm_utils_ascii_str_to_int64); g_test_add_func ("/general/nm_utils_ip6_address_clear_host_address", test_nm_utils_ip6_address_clear_host_address); From 1f8418541ecb36c2d1e6222386b464c23e326fe3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Feb 2014 12:54:36 +0100 Subject: [PATCH 3/4] core: add nm_utils_kill_child_async() and nm_utils_kill_child_sync() function Add utility function to kill and reap a child process. https://bugzilla.gnome.org/show_bug.cgi?id=725660 Signed-off-by: Thomas Haller --- .gitignore | 1 + src/NetworkManagerUtils.c | 422 +++++++++++++++++++++++++++ src/NetworkManagerUtils.h | 8 + src/tests/Makefile.am | 11 +- src/tests/test-general-with-expect.c | 342 ++++++++++++++++++++++ 5 files changed, 783 insertions(+), 1 deletion(-) create mode 100644 src/tests/test-general-with-expect.c diff --git a/.gitignore b/.gitignore index f215e02e96..34d3c28f26 100644 --- a/.gitignore +++ b/.gitignore @@ -182,6 +182,7 @@ valgrind-*.log /src/tests/test-dcb /src/tests/test-dhcp-options /src/tests/test-general +/src/tests/test-general-with-expect /src/tests/test-ip4-config /src/tests/test-ip6-config /src/tests/test-wifi-ap-utils diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 1093ae6c45..6b23d76dae 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -26,6 +26,8 @@ #include #include #include +#include +#include #include "NetworkManagerUtils.h" #include "nm-utils.h" @@ -144,6 +146,426 @@ nm_spawn_process (const char *args) return status; } +/******************************************************************************************/ + +typedef struct { + pid_t pid; + guint64 log_domain; + union { + struct { + gint64 wait_start_us; + guint source_timeout_kill_id; + } async; + struct { + gboolean success; + int child_status; + } sync; + }; + NMUtilsKillChildAsyncCb callback; + void *user_data; + + char log_name[1]; /* variable-length object, must be last element!! */ +} KillChildAsyncData; + +#define LOG_NAME_FMT "kill child process '%s' (%ld)" +#define LOG_NAME_ARGS log_name,(long)pid + +static KillChildAsyncData * +_kc_async_data_alloc (pid_t pid, guint64 log_domain, const char *log_name, NMUtilsKillChildAsyncCb callback, void *user_data) +{ + KillChildAsyncData *data; + size_t log_name_len; + + /* append the name at the end of our KillChildAsyncData. */ + log_name_len = strlen (LOG_NAME_FMT) + 20 + strlen (log_name); + data = g_malloc (sizeof (KillChildAsyncData) - 1 + log_name_len); + g_snprintf (data->log_name, log_name_len, LOG_NAME_FMT, LOG_NAME_ARGS); + + data->pid = pid; + data->user_data = user_data; + data->callback = callback; + data->log_domain = log_domain; + + return data; +} + +#define KC_EXIT_TO_STRING_BUF_SIZE 128 +static const char * +_kc_exit_to_string (char *buf, int exit) +#define _kc_exit_to_string(buf, exit) ( G_STATIC_ASSERT_EXPR(sizeof (buf) == KC_EXIT_TO_STRING_BUF_SIZE && sizeof ((buf)[0]) == 1), _kc_exit_to_string (buf, exit) ) +{ + if (WIFEXITED (exit)) + g_snprintf (buf, KC_EXIT_TO_STRING_BUF_SIZE, "normally with status %d", WEXITSTATUS (exit)); + else if (WIFSIGNALED (exit)) + g_snprintf (buf, KC_EXIT_TO_STRING_BUF_SIZE, "by signal %d", WTERMSIG (exit)); + else + g_snprintf (buf, KC_EXIT_TO_STRING_BUF_SIZE, "with unexpected status %d", exit); + return buf; +} + +static const char * +_kc_signal_to_string (int sig) +{ + switch (sig) { + case 0: return "no signal (0)"; + case SIGKILL: return "SIGKILL (" G_STRINGIFY (SIGKILL) ")"; + case SIGTERM: return "SIGTERM (" G_STRINGIFY (SIGTERM) ")"; + default: + return "Unexpected signal"; + } +} + +#define KC_WAITED_TO_STRING 100 +static const char * +_kc_waited_to_string (char *buf, gint64 wait_start_us) +#define _kc_waited_to_string(buf, wait_start_us) ( G_STATIC_ASSERT_EXPR(sizeof (buf) == KC_WAITED_TO_STRING && sizeof ((buf)[0]) == 1), _kc_waited_to_string (buf, wait_start_us) ) +{ + g_snprintf (buf, KC_WAITED_TO_STRING, " (%ld usec elapsed)", (long) (nm_utils_get_monotonic_timestamp_us () - wait_start_us)); + return buf; +} + +static void +_kc_cb_watch_child (GPid pid, gint status, gpointer user_data) +{ + KillChildAsyncData *data = user_data; + char buf_exit[KC_EXIT_TO_STRING_BUF_SIZE], buf_wait[KC_WAITED_TO_STRING]; + + if (data->async.source_timeout_kill_id) + g_source_remove (data->async.source_timeout_kill_id); + + nm_log_dbg (data->log_domain, "%s: terminated %s%s", + data->log_name, _kc_exit_to_string (buf_exit, status), + _kc_waited_to_string (buf_wait, data->async.wait_start_us)); + + if (data->callback) + data->callback (pid, TRUE, status, data->user_data); + + g_free (data); +} + +static gboolean +_kc_cb_timeout_grace_period (void *user_data) +{ + KillChildAsyncData *data = user_data; + int ret, errsv; + + data->async.source_timeout_kill_id = 0; + + if ((ret = kill (data->pid, SIGKILL)) != 0) { + errsv = errno; + /* ESRCH means, process does not exist or is already a zombie. */ + if (errsv != ESRCH) { + nm_log_err (LOGD_CORE | data->log_domain, "%s: kill(SIGKILL) returned unexpected return value %d: (%s, %d)", + data->log_name, ret, strerror (errsv), errsv); + } + } else { + nm_log_dbg (data->log_domain, "%s: process not terminated after %ld usec. Sending SIGKILL signal", + data->log_name, (long) (nm_utils_get_monotonic_timestamp_us () - data->async.wait_start_us)); + } + + return G_SOURCE_REMOVE; +} + +static gboolean +_kc_invoke_callback_idle (gpointer user_data) +{ + KillChildAsyncData *data = user_data; + + if (data->sync.success) { + char buf_exit[KC_EXIT_TO_STRING_BUF_SIZE]; + + nm_log_dbg (data->log_domain, "%s: invoke callback: terminated %s", + data->log_name, _kc_exit_to_string (buf_exit, data->sync.child_status)); + } else + nm_log_dbg (data->log_domain, "%s: invoke callback: killing child failed", data->log_name); + + data->callback (data->pid, data->sync.success, data->sync.child_status, data->user_data); + g_free (data); + + return G_SOURCE_REMOVE; +} + +static void +_kc_invoke_callback (pid_t pid, guint64 log_domain, const char *log_name, NMUtilsKillChildAsyncCb callback, void *user_data, gboolean success, int child_status) +{ + KillChildAsyncData *data; + + if (!callback) + return; + + data = _kc_async_data_alloc (pid, log_domain, log_name, callback, user_data); + data->sync.success = success; + data->sync.child_status = child_status; + + g_idle_add (_kc_invoke_callback_idle, data); +} + +/* nm_utils_kill_child_async: + * @pid: the process id of the process to kill + * @sig: signal to send initially. Set to 0 to send not signal. + * @log_domain: the logging domain used for logging (LOGD_NONE to suppress logging) + * @log_name: (allow-none): for logging, the name of the processes to kill + * @wait_before_kill_msec: Waittime in milliseconds before sending %SIGKILL signal. Set this value + * to zero, not to send %SIGKILL. If @sig is already %SIGKILL, this parameter is ignored. + * @callback: (allow-none): callback after the child terminated. This function will always + * be invoked asynchronously. + * @user_data: passed on to callback + * + * Uses g_child_watch_add(), so note the glib comment: if you obtain pid from g_spawn_async() or + * g_spawn_async_with_pipes() you will need to pass %G_SPAWN_DO_NOT_REAP_CHILD as flag to the spawn + * function for the child watching to work. + * Also note, that you must g_source_remove() any other child watchers for @pid because glib + * supports only one watcher per child. + **/ +void +nm_utils_kill_child_async (pid_t pid, int sig, guint64 log_domain, + const char *log_name, guint32 wait_before_kill_msec, + NMUtilsKillChildAsyncCb callback, void *user_data) +{ + int status = 0, errsv; + pid_t ret; + KillChildAsyncData *data; + char buf_exit[KC_EXIT_TO_STRING_BUF_SIZE]; + + g_return_if_fail (pid > 0); + g_return_if_fail (log_name != NULL); + + /* let's see if the child already terminated... */ + ret = waitpid (pid, &status, WNOHANG); + if (ret > 0) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": process %ld already terminated %s", + LOG_NAME_ARGS, (long) ret, _kc_exit_to_string (buf_exit, status)); + _kc_invoke_callback (pid, log_domain, log_name, callback, user_data, TRUE, status); + return; + } else if (ret != 0) { + errsv = errno; + /* ECHILD means, the process is not a child/does not exist or it has SIGCHILD blocked. */ + if (errsv != ECHILD) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": unexpected error while waitpid: %s (%d)", + LOG_NAME_ARGS, strerror (errsv), errsv); + _kc_invoke_callback (pid, log_domain, log_name, callback, user_data, FALSE, -1); + return; + } + } + + /* send the first signal. */ + if (kill (pid, sig) != 0) { + errsv = errno; + /* ESRCH means, process does not exist or is already a zombie. */ + if (errsv != ESRCH) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": unexpected error sending %s: %s (%d)", + LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv); + _kc_invoke_callback (pid, log_domain, log_name, callback, user_data, FALSE, -1); + return; + } + + /* let's try again with waitpid, probably there was a race... */ + ret = waitpid (pid, &status, 0); + if (ret > 0) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": process %ld already terminated %s", + LOG_NAME_ARGS, (long) ret, _kc_exit_to_string (buf_exit, status)); + _kc_invoke_callback (pid, log_domain, log_name, callback, user_data, TRUE, status); + } else { + errsv = errno; + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": failed due to unexpected return value %ld by waitpid (%s, %d) after sending %s", + LOG_NAME_ARGS, (long) ret, strerror (errsv), errsv, _kc_signal_to_string (sig)); + _kc_invoke_callback (pid, log_domain, log_name, callback, user_data, FALSE, -1); + } + return; + } + + data = _kc_async_data_alloc (pid, log_domain, log_name, callback, user_data); + data->async.wait_start_us = nm_utils_get_monotonic_timestamp_us (); + + if (sig != SIGKILL && wait_before_kill_msec > 0) { + data->async.source_timeout_kill_id = g_timeout_add (wait_before_kill_msec, _kc_cb_timeout_grace_period, data); + nm_log_dbg (log_domain, "%s: wait for process to terminate after sending %s (send SIGKILL in %ld milliseconds)...", + data->log_name, _kc_signal_to_string (sig), (long) wait_before_kill_msec); + } else { + data->async.source_timeout_kill_id = 0; + nm_log_dbg (log_domain, "%s: wait for process to terminate after sending %s...", + data->log_name, _kc_signal_to_string (sig)); + } + + g_child_watch_add (pid, _kc_cb_watch_child, data); +} + +/* nm_utils_kill_child_sync: + * @pid: process id to kill + * @sig: signal to sent initially. If 0, no signal is sent. If %SIGKILL, the + * second %SIGKILL signal is not sent after @wait_before_kill_msec milliseconds. + * @log_domain: log debug information for this domain. Errors and warnings are logged both + * as %LOGD_CORE and @log_domain. + * @log_name: (allow-none): name of the process to kill for logging. + * @child_status: (out) (allow-none): return the exit status of the child, if no error occured. + * @wait_before_kill_msec: Waittime in milliseconds before sending %SIGKILL signal. Set this value + * to zero, not to send %SIGKILL. If @sig is already %SIGKILL, this parameter has not effect. + * @sleep_duration_msec: the synchronous function sleeps repeatedly waiting for the child to terminate. + * Set to zero, to use the default (meaning 20 wakeups per seconds). + * + * Kill a child process synchronously and wait. The function first checks if the child already terminated + * and if it did, return the exit status. Otherwise send one @sig signal. @sig will always be + * sent unless the child already exited. If the child does not exit within @wait_before_kill_msec milliseconds, + * the function will send %SIGKILL and waits for the child indefinitly. If @wait_before_kill_msec is zero, no + * %SIGKILL signal will be sent. + **/ +gboolean +nm_utils_kill_child_sync (pid_t pid, int sig, guint64 log_domain, const char *log_name, + int *child_status, guint32 wait_before_kill_msec, + guint32 sleep_duration_msec) +{ + int status = 0, errsv; + pid_t ret; + gboolean success = FALSE; + gboolean was_waiting = FALSE, send_kill = FALSE; + char buf_exit[KC_EXIT_TO_STRING_BUF_SIZE]; + char buf_wait[KC_WAITED_TO_STRING]; + gint64 wait_start_us; + + g_return_val_if_fail (pid > 0, FALSE); + g_return_val_if_fail (log_name != NULL, FALSE); + + /* check if the child process already terminated... */ + ret = waitpid (pid, &status, WNOHANG); + if (ret > 0) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": process %ld already terminated %s", + LOG_NAME_ARGS, (long) ret, _kc_exit_to_string (buf_exit, status)); + success = TRUE; + goto out; + } else if (ret != 0) { + errsv = errno; + /* ECHILD means, the process is not a child/does not exist or it has SIGCHILD blocked. */ + if (errsv != ECHILD) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": unexpected error while waitpid: %s (%d)", + LOG_NAME_ARGS, strerror (errsv), errsv); + goto out; + } + } + + /* send first signal @sig */ + if (kill (pid, sig) != 0) { + errsv = errno; + /* ESRCH means, process does not exist or is already a zombie. */ + if (errsv != ESRCH) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": failed to send %s: %s (%d)", + LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv); + } else { + /* let's try again with waitpid, probably there was a race... */ + ret = waitpid (pid, &status, 0); + if (ret > 0) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": process %ld already terminated %s", + LOG_NAME_ARGS, (long) ret, _kc_exit_to_string (buf_exit, status)); + success = TRUE; + } else { + errsv = errno; + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": failed due to unexpected return value %ld by waitpid (%s, %d) after sending %s", + LOG_NAME_ARGS, (long) ret, strerror (errsv), errsv, _kc_signal_to_string (sig)); + } + } + goto out; + } + + wait_start_us = nm_utils_get_monotonic_timestamp_us (); + + /* wait for the process to terminated... */ + if (sig != SIGKILL) { + gint64 wait_until, now; + gulong sleep_time, sleep_duration_usec; + int loop_count = 0; + + sleep_duration_usec = (sleep_duration_msec <= 0) ? (G_USEC_PER_SEC / 20) : MIN (G_MAXULONG, ((gint64) sleep_duration_msec) * 1000L); + wait_until = wait_before_kill_msec <= 0 ? 0 : wait_start_us + (((gint64) wait_before_kill_msec) * 1000L); + + while (TRUE) { + ret = waitpid (pid, &status, WNOHANG); + if (ret > 0) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": after sending %s, process %ld exited %s%s", + LOG_NAME_ARGS, _kc_signal_to_string (sig), (long) ret, _kc_exit_to_string (buf_exit, status), + was_waiting ? _kc_waited_to_string (buf_wait, wait_start_us) : ""); + success = TRUE; + goto out; + } + if (ret == -1) { + errsv = errno; + /* ECHILD means, the process is not a child/does not exist or it has SIGCHILD blocked. */ + if (errsv != ECHILD) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": after sending %s, waitpid failed with %s (%d)%s", + LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv, + was_waiting ? _kc_waited_to_string (buf_wait, wait_start_us) : ""); + goto out; + } + } + + if (!wait_until) + break; + + now = nm_utils_get_monotonic_timestamp_us (); + if (now >= wait_until) + break; + + if (!was_waiting) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": waiting up to %ld milliseconds for process to terminate normally after sending %s...", + LOG_NAME_ARGS, (long) MAX (wait_before_kill_msec, 0), _kc_signal_to_string (sig)); + was_waiting = TRUE; + } + + sleep_time = MIN (wait_until - now, sleep_duration_usec); + if (loop_count < 20) { + /* At the beginning we expect the process to die fast. + * Limit the sleep time, the limit doubles with every iteration. */ + sleep_time = MIN (sleep_time, (((guint64) 1) << loop_count++) * G_USEC_PER_SEC / 2000); + } + g_usleep (sleep_time); + } + + /* send SIGKILL, if called with @wait_before_kill_msec > 0 */ + if (wait_until) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": sending SIGKILL...", LOG_NAME_ARGS); + + send_kill = TRUE; + if (kill (pid, SIGKILL) != 0) { + errsv = errno; + /* ESRCH means, process does not exist or is already a zombie. */ + if (errsv != ESRCH) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": failed to send SIGKILL (after sending %s), %s (%d)", + LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv); + goto out; + } + } + } + } + + if (!was_waiting) { + nm_log_dbg (log_domain, LOG_NAME_FMT ": waiting for process to terminate after sending %s%s...", + LOG_NAME_ARGS, _kc_signal_to_string (sig), send_kill ? " and SIGKILL" : ""); + } + + /* block until the child terminates. */ + while ((ret = waitpid (pid, &status, 0)) <= 0) { + errsv = errno; + + if (errsv != EINTR) { + nm_log_err (LOGD_CORE | log_domain, LOG_NAME_FMT ": after sending %s%s, waitpid failed with %s (%d)%s", + LOG_NAME_ARGS, _kc_signal_to_string (sig), send_kill ? " and SIGKILL" : "", strerror (errsv), errsv, + _kc_waited_to_string (buf_wait, wait_start_us)); + goto out; + } + } + + nm_log_dbg (log_domain, LOG_NAME_FMT ": after sending %s%s, process %ld exited %s%s", + LOG_NAME_ARGS, _kc_signal_to_string (sig), send_kill ? " and SIGKILL" : "", (long) ret, + _kc_exit_to_string (buf_exit, status), _kc_waited_to_string (buf_wait, wait_start_us)); + success = TRUE; +out: + if (child_status) + *child_status = success ? status : -1; + return success; +} +#undef LOG_NAME_FMT +#undef LOG_NAME_ARGS + +/******************************************************************************************/ + gboolean nm_match_spec_string (const GSList *specs, const char *match) { diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index ace62d548a..06143673c6 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -59,6 +59,14 @@ str_if_set (const char *str, const char *fallback) return str ? str : fallback; } +typedef void (*NMUtilsKillChildAsyncCb) (pid_t pid, gboolean success, int child_status, void *user_data); +void nm_utils_kill_child_async (pid_t pid, int sig, guint64 log_domain, const char *log_name, + guint32 wait_before_kill_msec, + NMUtilsKillChildAsyncCb callback, void *user_data); +gboolean nm_utils_kill_child_sync (pid_t pid, int sig, guint64 log_domain, const char *log_name, + int *child_status, guint32 wait_before_kill_msec, + guint32 sleep_duration_msec); + gboolean nm_match_spec_string (const GSList *specs, const char *string); gboolean nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr); gboolean nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index a44c2ee3bd..87310a252a 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -17,6 +17,7 @@ AM_CPPFLAGS = \ noinst_PROGRAMS = \ test-dhcp-options \ test-general \ + test-general-with-expect \ test-ip4-config \ test-ip6-config \ test-dcb \ @@ -75,11 +76,19 @@ test_general_SOURCES = \ test_general_LDADD = \ $(top_builddir)/src/libNetworkManager.la +####### general-with-expect test ####### + +test_general_with_expect_SOURCES = \ + test-general-with-expect.c + +test_general_with_expect_LDADD = \ + $(top_builddir)/src/libNetworkManager.la + ####### secret agent interface test ####### EXTRA_DIST = test-secret-agent.py ########################################### -TESTS = test-dhcp-options test-ip4-config test-ip6-config test-dcb test-resolvconf-capture test-general +TESTS = test-dhcp-options test-ip4-config test-ip6-config test-dcb test-resolvconf-capture test-general test-general-with-expect diff --git a/src/tests/test-general-with-expect.c b/src/tests/test-general-with-expect.c new file mode 100644 index 0000000000..f9dedbfef9 --- /dev/null +++ b/src/tests/test-general-with-expect.c @@ -0,0 +1,342 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright (C) 2014 Red Hat, Inc. + * + */ + +#include +#include +#include +#include +#include +#include + +#include "NetworkManagerUtils.h" +#include "nm-logging.h" + +#include "nm-test-utils.h" + +/*******************************************/ + +struct test_nm_utils_kill_child_async_data +{ + GMainLoop *loop; + pid_t pid; + gboolean called; + gboolean expected_success; + const int *expected_child_status; +}; + +static void +test_nm_utils_kill_child_async_cb (pid_t pid, gboolean success, int child_status, void *user_data) +{ + struct test_nm_utils_kill_child_async_data *data = user_data; + + g_assert (success == !!data->expected_success); + g_assert (pid == data->pid); + if (data->expected_child_status) + g_assert_cmpint (*data->expected_child_status, ==, child_status); + if (!success) + g_assert_cmpint (child_status, ==, -1); + + data->called = TRUE; + + g_assert (data->loop); + g_main_loop_quit (data->loop); +} + +static gboolean +test_nm_utils_kill_child_async_fail_cb (void *user_data) +{ + g_assert_not_reached (); +} + +static void +test_nm_utils_kill_child_async_do (const char *name, pid_t pid, int sig, guint32 wait_before_kill_msec, gboolean expected_success, const int *expected_child_status) +{ + gboolean success; + struct test_nm_utils_kill_child_async_data data = { }; + int timeout_id; + + data.pid = pid; + data.expected_success = expected_success; + data.expected_child_status = expected_child_status; + + nm_utils_kill_child_async (pid, sig, LOGD_CORE, name, wait_before_kill_msec, test_nm_utils_kill_child_async_cb, &data); + g_assert (!data.called); + + timeout_id = g_timeout_add_seconds (5, test_nm_utils_kill_child_async_fail_cb, &data); + + data.loop = g_main_loop_new (NULL, FALSE); + g_main_run (data.loop); + + g_assert (data.called); + success = g_source_remove (timeout_id); + g_assert (success); + + g_main_destroy (data.loop); +} + +static void +test_nm_utils_kill_child_sync_do (const char *name, pid_t pid, int sig, guint32 wait_before_kill_msec, gboolean expected_success, const int *expected_child_status) +{ + gboolean success; + int child_status = -1; + + success = nm_utils_kill_child_sync (pid, sig, LOGD_CORE, name, &child_status, wait_before_kill_msec, 0); + g_assert (success == !!expected_success); + if (expected_child_status) + g_assert_cmpint (*expected_child_status, ==, child_status); + + g_test_assert_expected_messages (); +} + +static pid_t +test_nm_utils_kill_child_spawn (char **argv, gboolean do_not_reap_child) +{ + GError *error = NULL; + int success; + GPid child_pid; + + success = g_spawn_async (NULL, + argv, + NULL, + G_SPAWN_SEARCH_PATH | (do_not_reap_child ? G_SPAWN_DO_NOT_REAP_CHILD : 0), + NULL, + NULL, + &child_pid, + &error); + g_assert (success && !error); + return child_pid; +} + +static pid_t +test_nm_utils_kill_child_create_and_join_pgroup () +{ + int err, tmp = 0; + int pipefd[2]; + pid_t pgid; + + err = pipe (pipefd); + g_assert (err == 0); + + pgid = fork(); + g_assert (pgid >= 0); + + if (pgid == 0) { + /* child process... */ + close (pipefd[0]); + + err = setpgid (0, 0); + g_assert (err == 0); + + err = write (pipefd[1], &tmp, sizeof (tmp)); + g_assert (err == sizeof (tmp)); + + close (pipefd[1]); + exit (0); + } + + close (pipefd[1]); + + err = read (pipefd[0], &tmp, sizeof (tmp)); + g_assert (err == sizeof (tmp)); + + close (pipefd[0]); + + err = setpgid (0, pgid); + g_assert (err == 0); + + + do { + err = waitpid (pgid, &tmp, 0); + } while (err == -1 && errno == EINTR); + g_assert (err == pgid); + g_assert (WIFEXITED (tmp) && WEXITSTATUS(tmp) == 0); + + return pgid; +} + +#define TEST_TOKEN "nm_test_kill_child_process" + +static void +test_nm_utils_kill_child (void) +{ + int err; + GLogLevelFlags fatal_mask; + char *argv_watchdog[] = { + "sh", + "-c", + "sleep 4; " + "kill -KILL 0; #watchdog for #" TEST_TOKEN, + NULL, + }; + char *argv1[] = { + "sh", + "-c", + "sleep 100000; exit $? #" TEST_TOKEN, + NULL, + }; + char *argv2[] = { + "sh", + "-c", + "exit 47; #" TEST_TOKEN, + NULL, + }; + char *argv3[] = { + "sh", + "-c", + "trap \"exit 47\" TERM; while true; do :; done; #" TEST_TOKEN, + NULL, + }; + char *argv4[] = { + "sh", + "-c", + "trap \"while true; do :; done\" TERM; while true; do :; done; #" TEST_TOKEN, + NULL, + }; + pid_t gpid; + pid_t pid1a_1, pid1a_2, pid1a_3, pid2a, pid3a, pid4a; + pid_t pid1s_1, pid1s_2, pid1s_3, pid2s, pid3s, pid4s; + + const int expected_exit_47 = 12032; /* exit with status 47 */ + const int expected_signal_TERM = SIGTERM; + const int expected_signal_KILL = SIGKILL; + + gpid = test_nm_utils_kill_child_create_and_join_pgroup (); + + test_nm_utils_kill_child_spawn (argv_watchdog, FALSE); + + pid1s_1 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid1s_2 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid1s_3 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid2s = test_nm_utils_kill_child_spawn (argv2, TRUE); + pid3s = test_nm_utils_kill_child_spawn (argv3, TRUE); + pid4s = test_nm_utils_kill_child_spawn (argv4, TRUE); + + pid1a_1 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid1a_2 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid1a_3 = test_nm_utils_kill_child_spawn (argv1, TRUE); + pid2a = test_nm_utils_kill_child_spawn (argv2, TRUE); + pid3a = test_nm_utils_kill_child_spawn (argv3, TRUE); + pid4a = test_nm_utils_kill_child_spawn (argv4, TRUE); + + /* give processes time to start (and potentially block signals) ... */ + g_usleep (G_USEC_PER_SEC / 10); + + + fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-1' (*): waiting up to 500 milliseconds for process to terminate normally after sending SIGTERM (15)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-1' (*): after sending SIGTERM (15), process * exited by signal 15 (* usec elapsed)"); + test_nm_utils_kill_child_sync_do ("test-s-1-1", pid1s_1, SIGTERM, 1000 / 2, TRUE, &expected_signal_TERM); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-2' (*): waiting for process to terminate after sending SIGKILL (9)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-2' (*): after sending SIGKILL (9), process * exited by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_sync_do ("test-s-1-2", pid1s_2, SIGKILL, 1000 / 2, TRUE, &expected_signal_KILL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-3' (*): waiting up to 1 milliseconds for process to terminate normally after sending no signal (0)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-3' (*): sending SIGKILL..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-1-3' (*): after sending no signal (0) and SIGKILL, process * exited by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_sync_do ("test-s-1-3", pid1s_3, 0, 1, TRUE, &expected_signal_KILL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-2' (*): process * already terminated normally with status 47"); + test_nm_utils_kill_child_sync_do ("test-s-2", pid2s, SIGTERM, 1000 / 2, TRUE, &expected_exit_47); + + /* send invalid signal. */ + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*kill child process 'test-s-3-0' (*): failed to send Unexpected signal: Invalid argument (22)"); + test_nm_utils_kill_child_sync_do ("test-s-3-0", pid3s, -1, 0, FALSE, NULL); + + /* really kill pid3s */ + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-3-1' (*): waiting up to 500 milliseconds for process to terminate normally after sending SIGTERM (15)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-3-1' (*): after sending SIGTERM (15), process * exited normally with status 47 (* usec elapsed)"); + test_nm_utils_kill_child_sync_do ("test-s-3-1", pid3s, SIGTERM, 1000 / 2, TRUE, &expected_exit_47); + + /* pid3s should not be a valid process, hence the call should fail. Note, that there + * is a race here. */ + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*kill child process 'test-s-3-2' (*): failed due to unexpected return value -1 by waitpid (No child processes, 10) after sending no signal (0)"); + test_nm_utils_kill_child_sync_do ("test-s-3-2", pid3s, 0, 0, FALSE, NULL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-4' (*): waiting up to 1 milliseconds for process to terminate normally after sending SIGTERM (15)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-4' (*): sending SIGKILL..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-s-4' (*): after sending SIGTERM (15) and SIGKILL, process * exited by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_sync_do ("test-s-4", pid4s, SIGTERM, 1, TRUE, &expected_signal_KILL); + + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-1' (*): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 500 milliseconds)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-1' (*): terminated by signal 15 (* usec elapsed)"); + test_nm_utils_kill_child_async_do ("test-a-1-1", pid1a_1, SIGTERM, 1000 / 2, TRUE, &expected_signal_TERM); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-2' (*): wait for process to terminate after sending SIGKILL (9)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-2' (*): terminated by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_async_do ("test-a-1-2", pid1a_2, SIGKILL, 1000 / 2, TRUE, &expected_signal_KILL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-3' (*): wait for process to terminate after sending no signal (0) (send SIGKILL in 1 milliseconds)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-3' (*): process not terminated after * usec. Sending SIGKILL signal"); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-1-3' (*): terminated by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_async_do ("test-a-1-3", pid1a_3, 0, 1, TRUE, &expected_signal_KILL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-2' (*): process * already terminated normally with status 47"); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-2' (*): invoke callback: terminated normally with status 47"); + test_nm_utils_kill_child_async_do ("test-a-2", pid2a, SIGTERM, 1000 / 2, TRUE, &expected_exit_47); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*kill child process 'test-a-3-0' (*): unexpected error sending Unexpected signal: Invalid argument (22)"); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-3-0' (*): invoke callback: killing child failed"); + test_nm_utils_kill_child_async_do ("test-a-3-0", pid3a, -1, 1000 / 2, FALSE, NULL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-3-1' (*): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 500 milliseconds)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-3-1' (*): terminated normally with status 47 (* usec elapsed)"); + test_nm_utils_kill_child_async_do ("test-a-3-1", pid3a, SIGTERM, 1000 / 2, TRUE, &expected_exit_47); + + /* pid3a should not be a valid process, hence the call should fail. Note, that there + * is a race here. */ + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*kill child process 'test-a-3-2' (*): failed due to unexpected return value -1 by waitpid (No child processes, 10) after sending no signal (0)"); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-3-2' (*): invoke callback: killing child failed"); + test_nm_utils_kill_child_async_do ("test-a-3-2", pid3a, 0, 0, FALSE, NULL); + + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-4' (*): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 1 milliseconds)..."); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-4' (*): process not terminated after * usec. Sending SIGKILL signal"); + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_DEBUG, "*kill child process 'test-a-4' (*): terminated by signal 9 (* usec elapsed)"); + test_nm_utils_kill_child_async_do ("test-a-4", pid4a, SIGTERM, 1, TRUE, &expected_signal_KILL); + + err = setpgid (0, 0); + g_assert (err == 0); + + kill (-gpid, SIGKILL); + + g_log_set_always_fatal (fatal_mask); + + g_test_assert_expected_messages (); +} + + +/*******************************************/ + +NMTST_DEFINE (); + +int +main (int argc, char **argv) +{ + nmtst_init_assert_logging (&argc, &argv); + + nm_logging_setup ("DEBUG", "DEFAULT", NULL, NULL); + + g_test_add_func ("/general/nm_utils_kill_child", test_nm_utils_kill_child); + + return g_test_run (); +} + From ff3b7538578454bd8cd9a0800588631b3f6a609e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Feb 2014 11:58:06 +0100 Subject: [PATCH 4/4] core: use nm_utils_kill_child_async() and nm_utils_kill_child_sync() Signed-off-by: Thomas Haller --- src/devices/nm-device.c | 19 ++-------- src/devices/team/nm-device-team.c | 33 +----------------- src/dhcp-manager/nm-dhcp-client.c | 44 +++--------------------- src/dns-manager/nm-dns-plugin.c | 40 ++------------------- src/dnsmasq-manager/nm-dnsmasq-manager.c | 29 ++-------------- src/ppp-manager/nm-ppp-manager.c | 29 ++-------------- 6 files changed, 13 insertions(+), 181 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 97546a2b34..bc21035b5c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -2323,13 +2323,7 @@ aipd_cleanup (NMDevice *self) } if (priv->aipd_pid > 0) { - kill (priv->aipd_pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_AUTOIP4, "waiting for avahi-autoipd pid %d to exit", priv->aipd_pid); - waitpid (priv->aipd_pid, NULL, 0); - nm_log_dbg (LOGD_AUTOIP4, "avahi-autoip pid %d cleaned up", priv->aipd_pid); - + nm_utils_kill_child_sync (priv->aipd_pid, SIGKILL, LOGD_AUTOIP4, "avahi-autoipd", NULL, 0, 0); priv->aipd_pid = -1; } @@ -5349,16 +5343,7 @@ ip_check_gw_ping_cleanup (NMDevice *self) } if (priv->gw_ping.pid) { - guint count = 20; - int status; - - kill (priv->gw_ping.pid, SIGKILL); - do { - if (waitpid (priv->gw_ping.pid, &status, WNOHANG) != 0) - break; - g_usleep (G_USEC_PER_SEC / 20); - } while (count--); - + nm_utils_kill_child_async (priv->gw_ping.pid, SIGTERM, priv->gw_ping.log_domain, "ping", 1000, NULL, NULL); priv->gw_ping.pid = 0; } } diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index fa7be78843..f29ad5eaca 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -304,37 +304,6 @@ master_update_slave_connection (NMDevice *self, /******************************************************************/ -static gboolean -ensure_killed (gpointer data) -{ - int pid = GPOINTER_TO_INT (data); - - if (kill (pid, 0) == 0) - kill (pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_TEAM, "waiting for teamd pid %d to exit", pid); - waitpid (pid, NULL, 0); - nm_log_dbg (LOGD_TEAM, "teamd pid %d cleaned up", pid); - - return FALSE; -} - -static void -service_kill (int pid) -{ - if (kill (pid, SIGTERM) == 0) - g_timeout_add_seconds (2, ensure_killed, GINT_TO_POINTER (pid)); - else { - kill (pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_TEAM, "waiting for teamd pid %d to exit", pid); - waitpid (pid, NULL, 0); - nm_log_dbg (LOGD_TEAM, "teamd pid %d cleaned up", pid); - } -} - static void teamd_timeout_remove (NMDevice *dev) { @@ -362,7 +331,7 @@ teamd_cleanup (NMDevice *dev, gboolean device_state_failed) } if (priv->teamd_pid > 0) { - service_kill (priv->teamd_pid); + nm_utils_kill_child_async (priv->teamd_pid, SIGTERM, LOGD_TEAM, "teamd", 2000, NULL, NULL); priv->teamd_pid = 0; } diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c index 807a68e8b8..5c39fafa68 100644 --- a/src/dhcp-manager/nm-dhcp-client.c +++ b/src/dhcp-manager/nm-dhcp-client.c @@ -139,47 +139,11 @@ watch_cleanup (NMDHCPClient *self) void nm_dhcp_client_stop_pid (GPid pid, const char *iface) { - int i = 5; /* roughly 0.5 seconds */ + char *name = iface ? g_strdup_printf ("dhcp-client-%s", iface) : NULL; - g_return_if_fail (pid > 0); - - /* Tell it to quit; maybe it wants to send out a RELEASE message */ - kill (pid, SIGTERM); - - while (i-- > 0) { - gint child_status; - int ret; - - ret = waitpid (pid, &child_status, WNOHANG); - if (ret > 0) - break; - - if (ret == -1) { - /* Child already exited */ - if (errno == ECHILD) { - /* Was it really our child and it exited? */ - if (kill (pid, 0) < 0 && errno == ESRCH) - break; - } else { - /* Took too long; shoot it in the head */ - i = 0; - break; - } - } - g_usleep (G_USEC_PER_SEC / 10); - } - - if (i <= 0) { - if (iface) { - nm_log_warn (LOGD_DHCP, "(%s): DHCP client pid %d didn't exit, will kill it.", - iface, pid); - } - kill (pid, SIGKILL); - - nm_log_dbg (LOGD_DHCP, "waiting for DHCP client pid %d to exit", pid); - waitpid (pid, NULL, 0); - nm_log_dbg (LOGD_DHCP, "DHCP client pid %d cleaned up", pid); - } + nm_utils_kill_child_sync (pid, SIGTERM, LOGD_DHCP, name ? name : "dhcp-client", NULL, + 1000 / 2, 1000 / 20); + g_free (name); } static void diff --git a/src/dns-manager/nm-dns-plugin.c b/src/dns-manager/nm-dns-plugin.c index e85b2a097a..62a362105e 100644 --- a/src/dns-manager/nm-dns-plugin.c +++ b/src/dns-manager/nm-dns-plugin.c @@ -28,6 +28,7 @@ #include "nm-dns-plugin.h" #include "nm-logging.h" #include "nm-posix-signals.h" +#include "NetworkManagerUtils.h" typedef struct { gboolean disposed; @@ -196,29 +197,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 +207,7 @@ 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 { - 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); - } + nm_utils_kill_child_async (priv->pid, SIGTERM, LOGD_DNS, priv->progname, 2000, NULL, NULL); priv->pid = 0; g_free (priv->progname); priv->progname = NULL; diff --git a/src/dnsmasq-manager/nm-dnsmasq-manager.c b/src/dnsmasq-manager/nm-dnsmasq-manager.c index 21bbd23a51..c76b5fe5c5 100644 --- a/src/dnsmasq-manager/nm-dnsmasq-manager.c +++ b/src/dnsmasq-manager/nm-dnsmasq-manager.c @@ -33,6 +33,7 @@ #include "nm-glib-compat.h" #include "nm-posix-signals.h" #include "nm-utils.h" +#include "NetworkManagerUtils.h" typedef struct { char *iface; @@ -432,22 +433,6 @@ nm_dnsmasq_manager_start (NMDnsMasqManager *manager, return priv->pid > 0; } -static gboolean -ensure_killed (gpointer data) -{ - int pid = GPOINTER_TO_INT (data); - - if (kill (pid, 0) == 0) - kill (pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_SHARING, "waiting for dnsmasq pid %d to exit", pid); - waitpid (pid, NULL, 0); - nm_log_dbg (LOGD_SHARING, "dnsmasq pid %d cleaned up", pid); - - return FALSE; -} - void nm_dnsmasq_manager_stop (NMDnsMasqManager *manager) { @@ -463,17 +448,7 @@ nm_dnsmasq_manager_stop (NMDnsMasqManager *manager) } if (priv->pid) { - if (kill (priv->pid, SIGTERM) == 0) - g_timeout_add_seconds (2, ensure_killed, GINT_TO_POINTER (priv->pid)); - else { - kill (priv->pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_SHARING, "waiting for dnsmasq pid %d to exit", priv->pid); - waitpid (priv->pid, NULL, 0); - nm_log_dbg (LOGD_SHARING, "dnsmasq pid %d cleaned up", priv->pid); - } - + nm_utils_kill_child_async (priv->pid, SIGTERM, LOGD_SHARING, "dnsmasq", 2000, NULL, NULL); priv->pid = 0; } diff --git a/src/ppp-manager/nm-ppp-manager.c b/src/ppp-manager/nm-ppp-manager.c index 0101e1c24d..f7a743cc0b 100644 --- a/src/ppp-manager/nm-ppp-manager.c +++ b/src/ppp-manager/nm-ppp-manager.c @@ -42,6 +42,7 @@ #include #include "NetworkManager.h" +#include "NetworkManagerUtils.h" #include "nm-glib-compat.h" #include "nm-ppp-manager.h" #include "nm-setting-connection.h" @@ -1109,22 +1110,6 @@ out: return priv->pid > 0; } -static gboolean -ensure_killed (gpointer data) -{ - int pid = GPOINTER_TO_INT (data); - - if (kill (pid, 0) == 0) - kill (pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_PPP, "waiting for pppd pid %d to exit", pid); - waitpid (pid, NULL, 0); - nm_log_dbg (LOGD_PPP, "pppd pid %d cleaned up", pid); - - return FALSE; -} - static void _ppp_cleanup (NMPPPManager *manager) { @@ -1159,17 +1144,7 @@ _ppp_cleanup (NMPPPManager *manager) } if (priv->pid) { - if (kill (priv->pid, SIGTERM) == 0) - g_timeout_add_seconds (2, ensure_killed, GINT_TO_POINTER (priv->pid)); - else { - kill (priv->pid, SIGKILL); - - /* ensure the child is reaped */ - nm_log_dbg (LOGD_PPP, "waiting for pppd pid %d to exit", priv->pid); - waitpid (priv->pid, NULL, 0); - nm_log_dbg (LOGD_PPP, "pppd pid %d cleaned up", priv->pid); - } - + nm_utils_kill_child_async (priv->pid, SIGTERM, LOGD_PPP, "pppd", 2000, NULL, NULL); priv->pid = 0; } }