From 168ec113a880038a2aa144b9384850e82399cf0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 18 May 2018 14:37:14 +0200 Subject: core: add and use NM_SHUTDOWN_TIMEOUT_MS as duration that we plan for shutdown nm_ppp_manager_stop() wants to ensure that the pppd process is really gone. For that it uses nm_utils_kill_child_async() to first send SIGTERM, and sending SIGKILL after a timeout. Later, we want to fix shutdown of NetworkManager to iterate the mainloop during shutdown, so that such operations are still handled. However, we can only delay shutdown for a certain time. After a timeout (NM_SHUTDOWN_TIMEOUT_MS plus NM_SHUTDOWN_TIMEOUT_MS_GRACE) we really have to give up and terminate. That means, the right amount of time between sending SIGTERM and SIGKILL is exactly NM_SHUTDOWN_TIMEOUT_MS. Hopefully that is of course sufficient in the first place. If not, send SIGKILL afterwards, and give a bit more time (NM_SHUTDOWN_TIMEOUT_MS_GRACE) to reap the child. And if all this time is still not enough, something is really odd and we abort waiting, with a warning in the logfile. Since we don't properly handle shutdown yet, the description above is not really true. But with this patch, we fix it from point of view of NMPPPManager. --- src/NetworkManagerUtils.h | 19 +++++++++++++++++++ src/ppp/nm-ppp-manager.c | 3 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index b8d3a4f064..b26d08bdce 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -56,6 +56,25 @@ int nm_match_spec_device_by_pllink (const NMPlatformLink *pllink, /*****************************************************************************/ +/* during shutdown, there are two relevant timeouts. One is + * NM_SHUTDOWN_TIMEOUT_MS which is plenty of time, that we give for all + * actions to complete. Of course, during shutdown components should hurry + * to cleanup. + * + * When we initiate shutdown, we should start killing child processes + * with SIGTERM. If they don't complete within NM_SHUTDOWN_TIMEOUT_MS, we send + * SIGKILL. + * + * After NM_SHUTDOWN_TIMEOUT_MS, NetworkManager will however not yet terminate right + * away. It iterates the mainloop for another NM_SHUTDOWN_TIMEOUT_MS_EXTRA. This + * should give time to reap the child process (after SIGKILL). + * + * So, the maxiumum time we should wait before sending SIGKILL should be at most + * NM_SHUTDOWN_TIMEOUT_MS. + */ +#define NM_SHUTDOWN_TIMEOUT_MS 1500 +#define NM_SHUTDOWN_TIMEOUT_MS_WATCHDOG 500 + typedef struct _NMShutdownWaitObjHandle NMShutdownWaitObjHandle; NMShutdownWaitObjHandle *_nm_shutdown_wait_obj_register (GObject *watched_obj, diff --git a/src/ppp/nm-ppp-manager.c b/src/ppp/nm-ppp-manager.c index e7de5d4a2e..fc658bec93 100644 --- a/src/ppp/nm-ppp-manager.c +++ b/src/ppp/nm-ppp-manager.c @@ -1253,7 +1253,8 @@ _ppp_manager_stop (NMPPPManager *self, handle->shutdown_waitobj = g_object_new (G_TYPE_OBJECT, NULL); nm_shutdown_wait_obj_register (handle->shutdown_waitobj, "ppp-manager-wait-kill-pppd"); nm_utils_kill_child_async (nm_steal_int (&priv->pid), - SIGTERM, LOGD_PPP, "pppd", 2000, + SIGTERM, LOGD_PPP, "pppd", + NM_SHUTDOWN_TIMEOUT_MS, _stop_child_cb, handle); return handle; -- cgit v1.2.1