From fd8932ef6922c006b42206e0d83eafb1bdb52eb2 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 18 Feb 2022 11:27:58 +0100 Subject: utils: remove signal argument from nm_utils_kill_child_async() ...and rename it appropriately. We always call it with SIGTERM, so this simplifies thing a bit. --- src/core/devices/nm-device.c | 3 +- src/core/devices/team/nm-device-team.c | 3 +- src/core/dnsmasq/nm-dnsmasq-manager.c | 2 +- src/core/nm-core-utils.c | 62 +++++++++++++++++------------- src/core/nm-core-utils.h | 3 +- src/core/ppp/nm-ppp-manager.c | 5 +-- src/core/tests/test-core-with-expect.c | 70 ++++++++-------------------------- 7 files changed, 58 insertions(+), 90 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index bebfc4c3ab..b5531799a0 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -13370,8 +13370,7 @@ ip_check_gw_ping_cleanup(NMDevice *self) nm_clear_g_source(&priv->gw_ping.timeout); if (priv->gw_ping.pid) { - nm_utils_kill_child_async(priv->gw_ping.pid, - SIGTERM, + nm_utils_term_child_async(priv->gw_ping.pid, priv->gw_ping.log_domain, "ping", 1000, diff --git a/src/core/devices/team/nm-device-team.c b/src/core/devices/team/nm-device-team.c index 16cd2e8715..2b8005d089 100644 --- a/src/core/devices/team/nm-device-team.c +++ b/src/core/devices/team/nm-device-team.c @@ -287,8 +287,7 @@ teamd_cleanup(NMDeviceTeam *self, gboolean free_tdc) if (priv->teamd_pid > 0) { priv->kill_in_progress = TRUE; - nm_utils_kill_child_async(priv->teamd_pid, - SIGTERM, + nm_utils_term_child_async(priv->teamd_pid, LOGD_TEAM, "teamd", 2000, diff --git a/src/core/dnsmasq/nm-dnsmasq-manager.c b/src/core/dnsmasq/nm-dnsmasq-manager.c index 4ab91e7e33..88f8fc7261 100644 --- a/src/core/dnsmasq/nm-dnsmasq-manager.c +++ b/src/core/dnsmasq/nm-dnsmasq-manager.c @@ -299,7 +299,7 @@ nm_dnsmasq_manager_stop(NMDnsMasqManager *manager) nm_clear_g_source(&priv->dm_watch_id); if (priv->pid) { - nm_utils_kill_child_async(priv->pid, SIGTERM, LOGD_SHARING, "dnsmasq", 2000, NULL, NULL); + nm_utils_term_child_async(priv->pid, LOGD_SHARING, "dnsmasq", 2000, NULL, NULL); priv->pid = 0; } diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index e05638e43c..cb62f61c34 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -461,31 +461,14 @@ _kc_invoke_callback(pid_t pid, nm_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: 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, - NMLogDomain log_domain, - const char *log_name, - guint32 wait_before_kill_msec, - NMUtilsKillChildAsyncCb callback, - void *user_data) +static void +_kill_child_async(pid_t pid, + int sig, + NMLogDomain log_domain, + const char *log_name, + guint32 wait_before_kill_msec, + NMUtilsKillChildAsyncCb callback, + void *user_data) { int status = 0, errsv; pid_t ret; @@ -582,6 +565,33 @@ nm_utils_kill_child_async(pid_t pid, g_child_watch_add(pid, _kc_cb_watch_child, data); } +/* nm_utils_term_child_async: + * @pid: the process id of the process to kill + * @log_domain: the logging domain used for logging (LOGD_NONE to suppress logging) + * @log_name: 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_term_child_async(pid_t pid, + NMLogDomain log_domain, + const char *log_name, + guint32 wait_before_kill_msec, + NMUtilsKillChildAsyncCb callback, + void *user_data) +{ + _kill_child_async(pid, SIGTERM, log_domain, log_name, wait_before_kill_msec, callback, user_data); +} + static gulong _sleep_duration_convert_ms_to_us(guint32 sleep_duration_msec) { @@ -4887,7 +4897,7 @@ helper_info_free(gpointer data) if (info->pid != -1) { nm_assert(info->pid > 1); - nm_utils_kill_child_async(info->pid, SIGKILL, LOGD_CORE, _NMLOG_PREFIX_NAME, 0, NULL, NULL); + _kill_child_async(info->pid, SIGKILL, LOGD_CORE, _NMLOG_PREFIX_NAME, 0, NULL, NULL); } g_free(info); diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index 7079ab2f25..305544c89a 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -164,8 +164,7 @@ 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, +void nm_utils_term_child_async(pid_t pid, guint64 log_domain, const char *log_name, guint32 wait_before_kill_msec, diff --git a/src/core/ppp/nm-ppp-manager.c b/src/core/ppp/nm-ppp-manager.c index f1f1030575..d762030869 100644 --- a/src/core/ppp/nm-ppp-manager.c +++ b/src/core/ppp/nm-ppp-manager.c @@ -1242,8 +1242,7 @@ _ppp_manager_stop(NMPPPManager *self, * that delays shutdown. */ handle->shutdown_waitobj = g_object_new(G_TYPE_OBJECT, NULL); nm_shutdown_wait_obj_register_object(handle->shutdown_waitobj, "ppp-manager-wait-kill-pppd"); - nm_utils_kill_child_async(nm_steal_int(&priv->pid), - SIGTERM, + nm_utils_term_child_async(nm_steal_int(&priv->pid), LOGD_PPP, "pppd", NM_SHUTDOWN_TIMEOUT_MS, @@ -1269,7 +1268,7 @@ _ppp_manager_stop_cancel(NMPPPManagerStopHandle *handle) /* a real handle. Only invoke the callback (synchronously). This marks * the handle as handled, but it keeps shutdown_waitobj around, until - * nm_utils_kill_child_async() returns. */ + * nm_utils_term_child_async() returns. */ _stop_handle_complete(handle, TRUE); } diff --git a/src/core/tests/test-core-with-expect.c b/src/core/tests/test-core-with-expect.c index eca098c438..09a6f3744d 100644 --- a/src/core/tests/test-core-with-expect.c +++ b/src/core/tests/test-core-with-expect.c @@ -60,7 +60,7 @@ test_nm_utils_monotonic_timestamp_as_boottime(void) /*****************************************************************************/ -struct test_nm_utils_kill_child_async_data { +struct test_nm_utils_term_child_async_data { GMainLoop *loop; pid_t pid; gboolean called; @@ -69,9 +69,9 @@ struct test_nm_utils_kill_child_async_data { }; static void -test_nm_utils_kill_child_async_cb(pid_t pid, gboolean success, int child_status, void *user_data) +test_nm_utils_term_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; + struct test_nm_utils_term_child_async_data *data = user_data; g_assert(success == !!data->expected_success); g_assert(pid == data->pid); @@ -87,37 +87,35 @@ test_nm_utils_kill_child_async_cb(pid_t pid, gboolean success, int child_status, } static gboolean -test_nm_utils_kill_child_async_fail_cb(void *user_data) +test_nm_utils_term_child_async_fail_cb(void *user_data) { g_assert_not_reached(); } static void -test_nm_utils_kill_child_async_do(const char *name, +test_nm_utils_term_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 = {}; + struct test_nm_utils_term_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, + nm_utils_term_child_async(pid, LOGD_CORE, name, wait_before_kill_msec, - test_nm_utils_kill_child_async_cb, + test_nm_utils_term_child_async_cb, &data); g_assert(!data.called); - timeout_id = g_timeout_add_seconds(5, test_nm_utils_kill_child_async_fail_cb, &data); + timeout_id = g_timeout_add_seconds(5, test_nm_utils_term_child_async_fail_cb, &data); data.loop = g_main_loop_new(NULL, FALSE); g_main_loop_run(data.loop); @@ -258,7 +256,7 @@ do_test_nm_utils_kill_child(void) "trap \"while true; do :; done\" TERM; while true; do :; done; #" TEST_TOKEN, NULL, }; - pid_t pid1a_1, pid1a_2, pid1a_3, pid2a, pid3a, pid4a; + pid_t pid1a_1, pid2a, pid4a; pid_t pid1s_1, pid1s_2, pid1s_3, pid2s, pid3s, pid4s; const int expected_exit_47 = 12032; /* exit with status 47 */ @@ -275,10 +273,7 @@ do_test_nm_utils_kill_child(void) 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) ... */ @@ -349,59 +344,26 @@ do_test_nm_utils_kill_child(void) "after sending SIGTERM (15) (send SIGKILL in 3000 milliseconds)..."); NMTST_EXPECT_NM_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", + test_nm_utils_term_child_async_do("test-a-1-1", pid1a_1, - SIGTERM, 3000, TRUE, &expected_signal_TERM); - NMTST_EXPECT_NM_DEBUG("kill child process 'test-a-1-2' (*): wait for process to terminate " - "after sending SIGKILL (9)..."); - NMTST_EXPECT_NM_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); - - NMTST_EXPECT_NM_DEBUG("kill child process 'test-a-1-3' (*): wait for process to terminate " - "after sending no signal (0) (send SIGKILL in 1 milliseconds)..."); - NMTST_EXPECT_NM_DEBUG("kill child process 'test-a-1-3' (*): process not terminated after * " - "usec. Sending SIGKILL signal"); - NMTST_EXPECT_NM_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); - NMTST_EXPECT_NM_DEBUG( "kill child process 'test-a-2' (*): process * already terminated normally with status 47"); NMTST_EXPECT_NM_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, 3000, TRUE, &expected_exit_47); + test_nm_utils_term_child_async_do("test-a-2", pid2a, 3000, TRUE, &expected_exit_47); - NMTST_EXPECT_NM_ERROR("kill child process 'test-a-3-0' (*): unexpected error sending " - "Unexpected signal: Invalid argument (22)"); - NMTST_EXPECT_NM_DEBUG( - "kill child process 'test-a-3-0' (*): invoke callback: killing child failed"); - /* coverity[negative_returns] */ - test_nm_utils_kill_child_async_do("test-a-3-0", pid3a, -1, 1000 / 2, FALSE, NULL); - - NMTST_EXPECT_NM_DEBUG("kill child process 'test-a-3-1' (*): wait for process to terminate " - "after sending SIGTERM (15) (send SIGKILL in 3000 milliseconds)..."); - NMTST_EXPECT_NM_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, 3000, TRUE, &expected_exit_47); - - /* pid3a should not be a valid process, hence the call should fail. Note, that there + /* pid3s should not be a valid process, hence the call should fail. Note, that there * is a race here. */ NMTST_EXPECT_NM_ERROR( "kill child process 'test-a-3-2' (*): failed due to unexpected return value -1 by waitpid " - "(No child process*, 10) after sending no signal (0)"); + "(No child process*, 10) after sending SIGTERM (15)"); NMTST_EXPECT_NM_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); + test_nm_utils_term_child_async_do("test-a-3-2", pid3s, 0, FALSE, NULL); NMTST_EXPECT_NM_DEBUG("kill child process 'test-a-4' (*): wait for process to terminate after " "sending SIGTERM (15) (send SIGKILL in 1 milliseconds)..."); @@ -409,7 +371,7 @@ do_test_nm_utils_kill_child(void) "Sending SIGKILL signal"); NMTST_EXPECT_NM_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); + test_nm_utils_term_child_async_do("test-a-4", pid4a, 1, TRUE, &expected_signal_KILL); g_log_set_always_fatal(fatal_mask); -- cgit v1.2.3