From 06713e764572e4c2530e64c8d736799256961e9a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 16:53:34 +0200 Subject: glib-aux: add nm_g_main_context_iterate_for_msec() helper --- src/libnm-glib-aux/nm-shared-utils.c | 30 ++++++++++++++++++++++++++++++ src/libnm-glib-aux/nm-shared-utils.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index f2483da1ad..22354d17df 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -6548,3 +6548,33 @@ nm_utils_thread_local_register_destroy(gpointer tls_data, GDestroyNotify destroy entry->destroy_notify = destroy_notify; c_list_link_tail(lst_head, &entry->lst); } + +/*****************************************************************************/ + +static gboolean +_iterate_for_msec_timeout(gpointer user_data) +{ + GSource **p_source = user_data; + + nm_clear_g_source_inst(p_source); + return G_SOURCE_CONTINUE; +} + +void +nm_g_main_context_iterate_for_msec(GMainContext *context, guint timeout_msec) +{ + GSource *source; + + /* In production is this function not very useful. It is however useful to + * have in the toolbox for printf debugging. */ + + source = g_timeout_source_new(timeout_msec); + g_source_set_callback(source, _iterate_for_msec_timeout, &source, NULL); + + if (!context) + context = g_main_context_default(); + + g_source_attach(source, context); + while (source) + g_main_context_iteration(context, TRUE); +} diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index fd7f7dc309..975897602d 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -1901,6 +1901,8 @@ nm_g_main_context_iterate_ready(GMainContext *context) } } +void nm_g_main_context_iterate_for_msec(GMainContext *context, guint timeout_msec); + /*****************************************************************************/ static inline int -- cgit v1.2.1 From 0aaaab07d1ac3f68c5089612c21e71c1c2e7778f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 16:08:40 +0200 Subject: nm-sudo: fix clearing timeout source in _idle_timeout_cb() Fixes: f137b32d3117 ('sudo: introduce nm-sudo D-Bus service') --- src/nm-sudo/nm-sudo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c index d8cf42f3c8..c9f69af586 100644 --- a/src/nm-sudo/nm-sudo.c +++ b/src/nm-sudo/nm-sudo.c @@ -399,6 +399,7 @@ _idle_timeout_cb(gpointer user_data) GlobalData *gl = user_data; _LOGT("idle-timeout: expired"); + nm_clear_g_source_inst(&gl->source_idle_timeout); gl->is_shutting_down_timeout = TRUE; return G_SOURCE_CONTINUE; } -- cgit v1.2.1 From 31c48ec616576bf97242d5735cdce142339bacf8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 10:50:37 +0200 Subject: nm-sudo: reject new request once we have no well-known-name If we fail to acquire the well-known name or if we already released it, we must not accept anymore new requests. Otherwise, requests directly targeted to the unique name will keep the process alive, and prevent it from restarting (and serving the well-known name). Clients really should not talk to the unique name of a service that exits on idle. If they do, and the service is about to shut down, then the request will be rejected. After we released the name, there is now turning back and we should quit fast (only processing the requests we already have). Also, if we receive a SIGTERM, then we are requested to quit and should do so in a timely manner. That means, we will start with releasing the name. As the service is D-Bus activated, new requests can be served by the next instance (or if the service is about to be disabled altogether, they will start failing). --- src/nm-sudo/nm-sudo.c | 121 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 82 insertions(+), 39 deletions(-) diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c index c9f69af586..a5d98e8eb4 100644 --- a/src/nm-sudo/nm-sudo.c +++ b/src/nm-sudo/nm-sudo.c @@ -56,6 +56,8 @@ struct _GlobalData { * of the request, so it is ONLY for testing. */ bool no_auth_for_testing; + bool reject_new_requests; + bool is_shutting_down_quitting; bool is_shutting_down_timeout; bool is_shutting_down_cleanup; @@ -283,6 +285,23 @@ _bus_method_call(GDBusConnection * connection, return; } + if (gl->reject_new_requests) { + /* after the name was released, we must not accept new requests. This new + * request was probably targeted against the unique-name. But we already + * gave up the well-known name. If we'd accept new request now, they would + * keep the service running indefinitely (and thus preventing the service + * to restart and serve the well-known name. */ + _LOGT("dbus: request sender=%s, %s%s, SERVER SHUTTING DOWN", + sender, + method_name, + g_variant_get_type_string(parameters)); + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_NO_SERVER, + "Server is exiting"); + return; + } + _pending_job_register_object(gl, G_OBJECT(invocation)); _LOGT("dbus: request sender=%s, %s%s", @@ -476,11 +495,59 @@ _pending_job_register_object(GlobalData *gl, GObject *obj) static void _bus_release_name_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - _nm_unused gs_unref_object GObject *keep_alive_object = user_data; + _nm_unused gs_unref_object GObject *keep_alive_object = NULL; + GlobalData * gl; + nm_utils_user_data_unpack(user_data, &gl, &keep_alive_object); + + gl->reject_new_requests = TRUE; g_main_context_wakeup(NULL); } +static gboolean +_bus_release_name(GlobalData *gl) +{ + gs_unref_object GObject *keep_alive_object = NULL; + int r; + + /* We already requested a name. To exit-on-idle without race, we need to dance. + * See https://lists.freedesktop.org/archives/dbus/2015-May/016671.html . */ + + if (!gl->name_requested) + return FALSE; + + gl->name_requested = FALSE; + gl->is_shutting_down_quitting = TRUE; + + _LOGT("shutdown: release-name"); + + keep_alive_object = g_object_new(G_TYPE_OBJECT, NULL); + + /* we use the _pending_job_register_object() mechanism to make the loop busy during + * shutdown. */ + _pending_job_register_object(gl, keep_alive_object); + + r = nm_sd_notify("STOPPING=1"); + if (r < 0) + _LOGW("shutdown: sd_notifiy(STOPPING=1) failed: %s", nm_strerror_native(-r)); + else + _LOGT("shutdown: sd_notifiy(STOPPING=1) succeeded"); + + g_dbus_connection_call(gl->dbus_connection, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "ReleaseName", + g_variant_new("(s)", NM_SUDO_DBUS_BUS_NAME), + G_VARIANT_TYPE("(u)"), + G_DBUS_CALL_FLAGS_NONE, + 10000, + NULL, + _bus_release_name_cb, + nm_utils_user_data_pack(gl, g_steal_pointer(&keep_alive_object))); + return TRUE; +} + /*****************************************************************************/ static void @@ -555,9 +622,21 @@ main(int argc, char **argv) if (!g_cancellable_is_cancelled(gl->quit_cancellable)) exit_code = EXIT_FAILURE; gl->is_shutting_down_quitting = TRUE; + + if (gl->name_requested) { + /* We requested a name, but something went wrong. Below we will release + * the name right away. */ + } else { + /* In case we didn't even went as far to request the name. New requests + * can only come via the unique name, and as we are shutting down, they + * are rejected. */ + gl->reject_new_requests = TRUE; + } } while (TRUE) { + if (gl->is_shutting_down_quitting) + _bus_release_name(gl); if (!c_list_is_empty(&gl->pending_jobs_lst_head)) { /* we must first reply to all requests. No matter what. */ } else if (gl->is_shutting_down_quitting || gl->is_shutting_down_timeout) { @@ -565,44 +644,8 @@ main(int argc, char **argv) * if we received an idle-timeout and the very moment afterwards * a new request, then _bus_method_call() will clear gl->is_shutting_down_timeout * (via _pending_job_register_object()). */ - - if (gl->name_requested) { - gs_unref_object GObject *keep_alive_object = g_object_new(G_TYPE_OBJECT, NULL); - - /* We already requested a name. To exit-on-idle without race, we need to dance. - * See https://lists.freedesktop.org/archives/dbus/2015-May/016671.html . */ - - gl->name_requested = FALSE; - gl->is_shutting_down_quitting = TRUE; - - _LOGT("shutdown: release-name"); - - /* we use the _pending_job_register_object() mechanism to make the loop busy during - * shutdown. */ - _pending_job_register_object(gl, keep_alive_object); - - r = nm_sd_notify("STOPPING=1"); - if (r < 0) - _LOGW("shutdown: sd_notifiy(STOPPING=1) failed: %s", nm_strerror_native(-r)); - else - _LOGT("shutdown: sd_notifiy(STOPPING=1) succeeded"); - - g_dbus_connection_call(gl->dbus_connection, - DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "ReleaseName", - g_variant_new("(s)", NM_SUDO_DBUS_BUS_NAME), - G_VARIANT_TYPE("(u)"), - G_DBUS_CALL_FLAGS_NONE, - 10000, - NULL, - _bus_release_name_cb, - g_steal_pointer(&keep_alive_object)); - continue; - } - - break; + if (!_bus_release_name(gl)) + break; } g_main_context_iteration(NULL, TRUE); -- cgit v1.2.1 From 9f0984c63b4279cb50818e7545fa45a5c7f991ce Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 11:22:54 +0200 Subject: nm-sudo: don't register pending job for current operations Currently we only implmement two operations (Ping() and GetFD()). Both complete right away. There is no need to register a pending job, if the job does not get processed asynchronously. In the future, we may have methods that need asynchronous processing and where we need to register them as pending job. --- src/nm-sudo/nm-sudo.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c index a5d98e8eb4..975d69b4a0 100644 --- a/src/nm-sudo/nm-sudo.c +++ b/src/nm-sudo/nm-sudo.c @@ -302,8 +302,6 @@ _bus_method_call(GDBusConnection * connection, return; } - _pending_job_register_object(gl, G_OBJECT(invocation)); - _LOGT("dbus: request sender=%s, %s%s", sender, method_name, -- cgit v1.2.1 From 412b5b4fa78fd788f539c8e304faccc82d3d83a2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 11:33:46 +0200 Subject: dispatcher: reject new requests after releasing name After we released the well-known name (or if we failed to ever request it), we must exit as fast as possible, so that a new instance can be started to serve new requests. At that point, reject new requests because they are targeted against the unique name, which they should not do (when talking to a D-Bus activated service that exits on idle, it's important to talk to the well-known name). Also, if we receive SIGTERM, start releasing the name. We are told to shut down, and must do so in a timely manner. Again, new requests shall not be served by this instance. --- src/nm-dispatcher/nm-dispatcher.c | 100 ++++++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 37 deletions(-) diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index b96cc82ea9..dba726e4e0 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -47,6 +47,8 @@ static struct { bool name_requested; + bool reject_new_requests; + bool exit_with_failure; bool shutdown_timeout; @@ -886,6 +888,13 @@ _method_call(GDBusConnection * connection, GDBusMethodInvocation *invocation, gpointer user_data) { + if (gl.reject_new_requests) { + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_NO_SERVER, + "Server is exiting"); + return; + } if (nm_streq(interface_name, NM_DISPATCHER_DBUS_INTERFACE)) { if (nm_streq(method_name, "Action")) { _method_call_action(invocation, parameters); @@ -1060,14 +1069,60 @@ signal_handler(gpointer user_data) return G_SOURCE_CONTINUE; } +/*****************************************************************************/ + static void _bus_release_name_cb(GObject *source, GAsyncResult *result, gpointer user_data) { nm_assert(gl.num_requests_pending > 0); + gl.reject_new_requests = TRUE; gl.num_requests_pending--; g_main_context_wakeup(NULL); } +static gboolean +_bus_release_name(void) +{ + int r; + + /* We already requested a name. To exit-on-idle without race, we need to dance. + * See https://lists.freedesktop.org/archives/dbus/2015-May/016671.html . */ + + if (!gl.name_requested) + return FALSE; + + gl.name_requested = FALSE; + gl.shutdown_quitting = TRUE; + + _LOG_X_T("shutdown: release-name"); + + /* we create a fake pending request. */ + gl.num_requests_pending++; + nm_clear_g_source_inst(&gl.quit_source); + + r = nm_sd_notify("STOPPING=1"); + if (r < 0) + _LOG_X_W("shutdown: sd_notifiy(STOPPING=1) failed: %s", nm_strerror_native(-r)); + else + _LOG_X_T("shutdown: sd_notifiy(STOPPING=1) succeeded"); + + g_dbus_connection_call(gl.dbus_connection, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "ReleaseName", + g_variant_new("(s)", NM_DISPATCHER_DBUS_SERVICE), + G_VARIANT_TYPE("(u)"), + G_DBUS_CALL_FLAGS_NONE, + 10000, + NULL, + _bus_release_name_cb, + NULL); + return TRUE; +} + +/*****************************************************************************/ + static gboolean parse_command_line(int *p_argc, char ***p_argv, GError **error) { @@ -1173,49 +1228,20 @@ main(int argc, char **argv) if (!g_cancellable_is_cancelled(gl.quit_cancellable)) gl.exit_with_failure = TRUE; gl.shutdown_quitting = TRUE; + + if (!gl.name_requested) + gl.reject_new_requests = TRUE; } while (TRUE) { + if (gl.shutdown_quitting) + _bus_release_name(); + if (gl.num_requests_pending > 0) { /* while we have requests pending, we cannot stop processing them... */ } else if (gl.shutdown_timeout || gl.shutdown_quitting) { - if (gl.name_requested) { - int r; - - /* We already requested a name. To exit-on-idle without race, we need to dance. - * See https://lists.freedesktop.org/archives/dbus/2015-May/016671.html . */ - - gl.name_requested = FALSE; - gl.shutdown_quitting = TRUE; - - _LOG_X_T("shutdown: release-name"); - - /* we create a fake pending request. */ - gl.num_requests_pending++; - nm_clear_g_source_inst(&gl.quit_source); - - r = nm_sd_notify("STOPPING=1"); - if (r < 0) - _LOG_X_W("shutdown: sd_notifiy(STOPPING=1) failed: %s", nm_strerror_native(-r)); - else - _LOG_X_T("shutdown: sd_notifiy(STOPPING=1) succeeded"); - - g_dbus_connection_call(gl.dbus_connection, - DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "ReleaseName", - g_variant_new("(s)", NM_DISPATCHER_DBUS_SERVICE), - G_VARIANT_TYPE("(u)"), - G_DBUS_CALL_FLAGS_NONE, - 10000, - NULL, - _bus_release_name_cb, - NULL); - continue; - } - - break; + if (!_bus_release_name()) + break; } g_main_context_iteration(NULL, TRUE); -- cgit v1.2.1 From 2665fe23c2f93679adfaf4eec7f92be1481b1b20 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Aug 2021 11:53:30 +0200 Subject: nm-sudo,dispatcher: rename and refactor code to make them more similar nm-sudo and nm-dispatcher are very similar from a high level. Both are D-Bus activated services that exit on idle and all they do, is to provide a simple D-Bus API with no objects or properties. Hence it's not surprising that they follow the same structure. Rename the code to make them look more similar. --- src/nm-dispatcher/nm-dispatcher.c | 120 +++++++++++++++++++++----------------- src/nm-sudo/nm-sudo.c | 73 ++++++++++++----------- 2 files changed, 105 insertions(+), 88 deletions(-) diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index dba726e4e0..b175d8eed3 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -7,15 +7,15 @@ #include "libnm-client-aux-extern/nm-default-client.h" -#include +#include +#include #include -#include #include -#include -#include #include +#include #include -#include +#include +#include #include "libnm-core-aux-extern/nm-dispatcher-api.h" #include "libnm-glib-aux/nm-dbus-aux.h" @@ -33,31 +33,36 @@ typedef struct Request Request; -static struct { +typedef struct { GDBusConnection *dbus_connection; GCancellable * quit_cancellable; - bool log_verbose; - bool log_stdout; - gboolean persist; - GSource * quit_source; - guint request_id_counter; - guint dbus_regist_id; + + bool log_verbose; + bool log_stdout; + + GSource *source_idle_timeout; gint64 start_timestamp_msec; - bool name_requested; + guint request_id_counter; + guint service_regist_id; - bool reject_new_requests; + gboolean persist; + + Request *current_request; + GQueue * requests_waiting; + int num_requests_pending; bool exit_with_failure; + bool name_requested; + bool reject_new_requests; + bool shutdown_timeout; bool shutdown_quitting; +} GlobalData; - Request *current_request; - GQueue * requests_waiting; - int num_requests_pending; -} gl; +GlobalData gl; typedef struct { Request *request; @@ -206,18 +211,20 @@ request_free(Request *request) g_slice_free(Request, request); } +/*****************************************************************************/ + static gboolean -quit_timeout_cb(gpointer user_data) +_idle_timeout_cb(gpointer user_data) { - nm_clear_g_source_inst(&gl.quit_source); + nm_clear_g_source_inst(&gl.source_idle_timeout); gl.shutdown_timeout = TRUE; return G_SOURCE_CONTINUE; } static void -quit_timeout_reschedule(void) +_idle_timeout_restart(void) { - nm_clear_g_source_inst(&gl.quit_source); + nm_clear_g_source_inst(&gl.source_idle_timeout); if (gl.persist) return; @@ -228,9 +235,11 @@ quit_timeout_reschedule(void) if (gl.num_requests_pending > 0) return; - gl.quit_source = nm_g_timeout_add_source(10000, quit_timeout_cb, NULL); + gl.source_idle_timeout = nm_g_timeout_add_source(10000, _idle_timeout_cb, NULL); } +/*****************************************************************************/ + /** * next_request: * @@ -278,7 +287,7 @@ next_request(Request *request) * Checks if all the scripts for the request have terminated and in such case * it sends the D-Bus response and releases the request resources. * - * It also decreases @num_requests_pending and possibly does quit_timeout_reschedule(). + * It also decreases @num_requests_pending and possibly does _idle_timeout_restart(). */ static void complete_request(Request *request) @@ -317,7 +326,7 @@ complete_request(Request *request) nm_assert(gl.num_requests_pending > 0); if (--gl.num_requests_pending <= 0) { nm_assert(!gl.current_request && !g_queue_peek_head(gl.requests_waiting)); - quit_timeout_reschedule(); + _idle_timeout_restart(); } } @@ -695,7 +704,7 @@ script_must_wait(const char *path) } static void -_method_call_action(GDBusMethodInvocation *invocation, GVariant *parameters) +_handle_action(GDBusMethodInvocation *invocation, GVariant *parameters) { const char * action; gs_unref_variant GVariant *connection = NULL; @@ -813,7 +822,7 @@ _method_call_action(GDBusMethodInvocation *invocation, GVariant *parameters) gl.num_requests_pending++; gl.shutdown_timeout = FALSE; - nm_clear_g_source_inst(&gl.quit_source); + nm_clear_g_source_inst(&gl.source_idle_timeout); for (i = 0; i < request->scripts->len; i++) { ScriptInfo *s = g_ptr_array_index(request->scripts, i); @@ -859,7 +868,7 @@ _method_call_action(GDBusMethodInvocation *invocation, GVariant *parameters) } static void -_method_call_ping(GDBusMethodInvocation *invocation, GVariant *parameters) +_handle_ping(GDBusMethodInvocation *invocation, GVariant *parameters) { gs_free char *msg = NULL; gint64 running_msec; @@ -879,14 +888,14 @@ _method_call_ping(GDBusMethodInvocation *invocation, GVariant *parameters) } static void -_method_call(GDBusConnection * connection, - const char * sender, - const char * object_path, - const char * interface_name, - const char * method_name, - GVariant * parameters, - GDBusMethodInvocation *invocation, - gpointer user_data) +_bus_method_call(GDBusConnection * connection, + const char * sender, + const char * object_path, + const char * interface_name, + const char * method_name, + GVariant * parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) { if (gl.reject_new_requests) { g_dbus_method_invocation_return_error(invocation, @@ -897,11 +906,11 @@ _method_call(GDBusConnection * connection, } if (nm_streq(interface_name, NM_DISPATCHER_DBUS_INTERFACE)) { if (nm_streq(method_name, "Action")) { - _method_call_action(invocation, parameters); + _handle_action(invocation, parameters); return; } if (nm_streq(method_name, "Ping")) { - _method_call_ping(invocation, parameters); + _handle_ping(invocation, parameters); return; } } @@ -944,7 +953,7 @@ static gboolean _bus_register_service(void) { static const GDBusInterfaceVTable interface_vtable = { - .method_call = _method_call, + .method_call = _bus_method_call, }; gs_free_error GError * error = NULL; NMDBusConnectionCallBlockingData data = { @@ -953,7 +962,7 @@ _bus_register_service(void) gs_unref_variant GVariant *ret = NULL; guint32 ret_val; - gl.dbus_regist_id = + gl.service_regist_id = g_dbus_connection_register_object(gl.dbus_connection, NM_DISPATCHER_DBUS_PATH, interface_info, @@ -961,7 +970,7 @@ _bus_register_service(void) NULL, NULL, &error); - if (gl.dbus_regist_id == 0) { + if (gl.service_regist_id == 0) { _LOG_X_W("dbus: could not export dispatcher D-Bus interface %s: %s", NM_DISPATCHER_DBUS_PATH, error->message); @@ -1059,7 +1068,7 @@ logging_shutdown(void) } static gboolean -signal_handler(gpointer user_data) +_signal_callback_term(gpointer user_data) { if (!gl.shutdown_quitting) { gl.shutdown_quitting = TRUE; @@ -1098,7 +1107,7 @@ _bus_release_name(void) /* we create a fake pending request. */ gl.num_requests_pending++; - nm_clear_g_source_inst(&gl.quit_source); + nm_clear_g_source_inst(&gl.source_idle_timeout); r = nm_sd_notify("STOPPING=1"); if (r < 0) @@ -1124,7 +1133,7 @@ _bus_release_name(void) /*****************************************************************************/ static gboolean -parse_command_line(int *p_argc, char ***p_argv, GError **error) +_initial_setup(int *p_argc, char ***p_argv, GError **error) { GOptionContext *opt_ctx; gboolean arg_debug = FALSE; @@ -1170,6 +1179,8 @@ parse_command_line(int *p_argc, char ***p_argv, GError **error) return success; } +/*****************************************************************************/ + int main(int argc, char **argv) { @@ -1178,14 +1189,16 @@ main(int argc, char **argv) GSource * source_int = NULL; signal(SIGPIPE, SIG_IGN); - source_term = nm_g_unix_signal_add_source(SIGTERM, signal_handler, GINT_TO_POINTER(SIGTERM)); - source_int = nm_g_unix_signal_add_source(SIGINT, signal_handler, GINT_TO_POINTER(SIGINT)); + source_term = + nm_g_unix_signal_add_source(SIGTERM, _signal_callback_term, GINT_TO_POINTER(SIGTERM)); + source_int = + nm_g_unix_signal_add_source(SIGINT, _signal_callback_term, GINT_TO_POINTER(SIGINT)); gl.start_timestamp_msec = nm_utils_clock_gettime_msec(CLOCK_BOOTTIME); gl.quit_cancellable = g_cancellable_new(); - if (!parse_command_line(&argc, &argv, &error)) { + if (!_initial_setup(&argc, &argv, &error)) { _LOG_X_W("Error parsing command line arguments: %s", error->message); gl.exit_with_failure = TRUE; goto done; @@ -1219,7 +1232,7 @@ main(int argc, char **argv) gl.requests_waiting = g_queue_new(); - quit_timeout_reschedule(); + _idle_timeout_restart(); if (!_bus_register_service()) { /* we failed to start the D-Bus service, and will shut down. However, @@ -1248,19 +1261,19 @@ main(int argc, char **argv) } done: - nm_g_main_context_iterate_ready(NULL); - gl.shutdown_quitting = TRUE; g_cancellable_cancel(gl.quit_cancellable); nm_assert(gl.num_requests_pending == 0); - if (gl.dbus_regist_id != 0) - g_dbus_connection_unregister_object(gl.dbus_connection, nm_steal_int(&gl.dbus_regist_id)); + if (gl.service_regist_id != 0) { + g_dbus_connection_unregister_object(gl.dbus_connection, + nm_steal_int(&gl.service_regist_id)); + } nm_clear_pointer(&gl.requests_waiting, g_queue_free); - nm_clear_g_source_inst(&gl.quit_source); + nm_clear_g_source_inst(&gl.source_idle_timeout); if (gl.dbus_connection) { g_dbus_connection_flush_sync(gl.dbus_connection, NULL, NULL); @@ -1276,7 +1289,6 @@ done: nm_clear_g_source_inst(&source_term); nm_clear_g_source_inst(&source_int); - g_clear_object(&gl.quit_cancellable); return gl.exit_with_failure ? 1 : 0; diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c index 975d69b4a0..1e8c941fd0 100644 --- a/src/nm-sudo/nm-sudo.c +++ b/src/nm-sudo/nm-sudo.c @@ -5,12 +5,12 @@ #include #include "c-list/src/c-list.h" +#include "libnm-base/nm-sudo-utils.h" #include "libnm-glib-aux/nm-dbus-aux.h" #include "libnm-glib-aux/nm-io-utils.h" #include "libnm-glib-aux/nm-logging-base.h" #include "libnm-glib-aux/nm-shared-utils.h" #include "libnm-glib-aux/nm-time-utils.h" -#include "libnm-base/nm-sudo-utils.h" /* nm-sudo doesn't link with libnm-core nor libnm-base, but these headers * can be used independently. */ @@ -37,30 +37,35 @@ typedef struct { } PendingJobData; struct _GlobalData { - GCancellable * quit_cancellable; GDBusConnection *dbus_connection; - GSource * source_sigterm; + GCancellable * quit_cancellable; + + GSource *source_sigterm; CList pending_jobs_lst_head; GSource *source_idle_timeout; - char * name_owner; - guint name_owner_changed_id; - guint service_regist_id; - gint64 start_timestamp_msec; - guint32 timeout_msec; - bool name_owner_initialized; - bool name_requested; + + char *name_owner; + + gint64 start_timestamp_msec; + + guint name_owner_changed_id; + guint service_regist_id; + + guint32 timeout_msec; + + bool name_owner_initialized; /* This is controlled by $NM_SUDO_NO_AUTH_FOR_TESTING. It disables authentication * of the request, so it is ONLY for testing. */ bool no_auth_for_testing; + bool name_requested; bool reject_new_requests; - bool is_shutting_down_quitting; - bool is_shutting_down_timeout; - bool is_shutting_down_cleanup; + bool shutdown_quitting; + bool shutdown_timeout; }; /*****************************************************************************/ @@ -136,7 +141,7 @@ _signal_callback_term(gpointer user_data) _LOGD("sigterm received (%s)", c_list_is_empty(&gl->pending_jobs_lst_head) ? "quit mainloop" : "cancel operations"); - gl->is_shutting_down_quitting = TRUE; + gl->shutdown_quitting = TRUE; g_cancellable_cancel(gl->quit_cancellable); return G_SOURCE_CONTINUE; } @@ -417,7 +422,7 @@ _idle_timeout_cb(gpointer user_data) _LOGT("idle-timeout: expired"); nm_clear_g_source_inst(&gl->source_idle_timeout); - gl->is_shutting_down_timeout = TRUE; + gl->shutdown_timeout = TRUE; return G_SOURCE_CONTINUE; } @@ -426,10 +431,7 @@ _idle_timeout_restart(GlobalData *gl) { nm_clear_g_source_inst(&gl->source_idle_timeout); - if (gl->is_shutting_down_quitting) - return; - - if (gl->is_shutting_down_cleanup) + if (gl->shutdown_quitting) return; if (!c_list_is_empty(&gl->pending_jobs_lst_head)) @@ -475,7 +477,7 @@ _pending_job_register_object(GlobalData *gl, GObject *obj) PendingJobData *idle_data; /* if we just hit the timeout, we can ignore it. */ - gl->is_shutting_down_timeout = FALSE; + gl->shutdown_timeout = FALSE; if (nm_clear_g_source_inst(&gl->source_idle_timeout)) _LOGT("idle-timeout: suspend timeout for pending request"); @@ -514,8 +516,8 @@ _bus_release_name(GlobalData *gl) if (!gl->name_requested) return FALSE; - gl->name_requested = FALSE; - gl->is_shutting_down_quitting = TRUE; + gl->name_requested = FALSE; + gl->shutdown_quitting = TRUE; _LOGT("shutdown: release-name"); @@ -565,6 +567,8 @@ _initial_setup(GlobalData *gl) gl->source_sigterm = nm_g_unix_signal_add_source(SIGTERM, _signal_callback_term, gl); } +/*****************************************************************************/ + int main(int argc, char **argv) { @@ -619,7 +623,7 @@ main(int argc, char **argv) * Let's fake a shutdown signal, and still process the request below. */ if (!g_cancellable_is_cancelled(gl->quit_cancellable)) exit_code = EXIT_FAILURE; - gl->is_shutting_down_quitting = TRUE; + gl->shutdown_quitting = TRUE; if (gl->name_requested) { /* We requested a name, but something went wrong. Below we will release @@ -633,14 +637,15 @@ main(int argc, char **argv) } while (TRUE) { - if (gl->is_shutting_down_quitting) + if (gl->shutdown_quitting) _bus_release_name(gl); + if (!c_list_is_empty(&gl->pending_jobs_lst_head)) { /* we must first reply to all requests. No matter what. */ - } else if (gl->is_shutting_down_quitting || gl->is_shutting_down_timeout) { + } else if (gl->shutdown_quitting || gl->shutdown_timeout) { /* we either hit the idle timeout or received SIGTERM. Note that * if we received an idle-timeout and the very moment afterwards - * a new request, then _bus_method_call() will clear gl->is_shutting_down_timeout + * a new request, then _bus_method_call() will clear gl->shutdown_timeout * (via _pending_job_register_object()). */ if (!_bus_release_name(gl)) break; @@ -652,7 +657,7 @@ main(int argc, char **argv) done: _LOGD("shutdown: cleanup"); - gl->is_shutting_down_cleanup = TRUE; + gl->shutdown_quitting = TRUE; g_cancellable_cancel(gl->quit_cancellable); nm_assert(c_list_is_empty(&gl->pending_jobs_lst_head)); @@ -665,19 +670,19 @@ done: g_dbus_connection_signal_unsubscribe(gl->dbus_connection, nm_steal_int(&gl->name_owner_changed_id)); } - nm_clear_g_source_inst(&gl->source_sigterm); - nm_clear_g_source_inst(&gl->source_idle_timeout); - nm_clear_g_free(&gl->name_owner); - - nm_g_main_context_iterate_ready(NULL); if (gl->dbus_connection) { g_dbus_connection_flush_sync(gl->dbus_connection, NULL, NULL); g_clear_object(&gl->dbus_connection); - nm_g_main_context_iterate_ready(NULL); } - nm_clear_g_cancellable(&gl->quit_cancellable); + nm_g_main_context_iterate_ready(NULL); + + nm_clear_g_free(&gl->name_owner); + + nm_clear_g_source_inst(&gl->source_sigterm); + nm_clear_g_source_inst(&gl->source_idle_timeout); + g_clear_object(&gl->quit_cancellable); _LOGD("exit (%d)", exit_code); return exit_code; -- cgit v1.2.1