From a6f5dbb426a9fa76cc2914d0cb685d98369bbce7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Mar 2023 14:54:54 +0100 Subject: [PATCH] core/dbus: "RequestName" of NetworkManager D-Bus API later to fix race NetworkManager.service is "Type=dbus". Systemd takes that as indication for declaring the service as started when the D-Bus name is acquired. Currently, we acquire the name very early. The benefit is, that the service appears to start very fast. However, most the D-Bus API is not yet populated or ready to use. So if you order your service `After=NetworkManager.service`, then there is a race that NetworkManager might not yet be fully usable. Another benefit was that requesting a D-Bus name is atomic. That means, we could take that to ensure only one NetworkManager daemon was running. If we noticed that NetworkManager is already running, we would quit without doing anything. In practice, systemd already ensures that the daemon is not running in parallel. This was still useful for catching misuse when testing manually. This is now no longer done. We will notice a concurrent NetworkManager only very late, at which point we might have already broken things (e.g. rewrite wrong state files). Fix the race with `After=` by acquiring the name much later. Note that NetworkManager is pretty slow during initialization. This easily adds several hundreds of milliseconds to the startup. --- src/core/main.c | 3 +++ src/core/nm-dbus-manager.c | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 4c436a631b..4c7de6cd37 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -508,6 +508,9 @@ main(int argc, char *argv[]) nm_log_dbg(LOGD_CORE, "setting up local loopback"); nm_platform_link_change_flags(NM_PLATFORM_GET, 1, IFF_UP, TRUE); + if (!nm_dbus_manager_request_name_sync(nm_dbus_manager_get())) + goto done; + success = TRUE; if (configure_and_quit == FALSE) { diff --git a/src/core/nm-dbus-manager.c b/src/core/nm-dbus-manager.c index 5ecd9fe680..0bde5971d8 100644 --- a/src/core/nm-dbus-manager.c +++ b/src/core/nm-dbus-manager.c @@ -1421,6 +1421,11 @@ nm_dbus_manager_request_name_sync(NMDBusManager *self) priv = NM_DBUS_MANAGER_GET_PRIVATE(self); + if (priv->objmgr_registration_id == 0) { + /* Do nothing. We're presumably in the configure-and-quit mode. */ + return TRUE; + } + g_return_val_if_fail(G_IS_DBUS_CONNECTION(priv->main_dbus_connection), FALSE); ret = g_dbus_connection_call_sync( @@ -1499,9 +1504,6 @@ nm_dbus_manager_setup(NMDBusManager *self) _LOGD("D-Bus connection created and ObjectManager object registered"); - if (!nm_dbus_manager_request_name_sync(self)) - return FALSE; - return TRUE; }