From b7911bae516a5b4cbacd20886e33b4a63597d1e4 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 31 Jul 2015 13:00:22 -0400 Subject: [PATCH] core: better order the code at startup NM was calling nm_bus_manager_start_service() to claim its bus name before it exported any of its objects, but this didn't matter under dbus-glib, because no client connections would be accepted until the main loop was started later on, by which point we would have exported everything. But with gdbus, method calls are initially received in the gdbus worker thread, which means that clients would be able to connect right away and then be told that the expected interfaces don't exist. So move the nm_bus_manager_start_service() call to occur after creating NMSettings and NMManager (and, indirectly, NMAgentManager). This requires splitting out the slow parts of nm_settings_new() into a new nm_settings_start(), so that we can create and export it first, and then read the connections, etc afterward. (Likewise, there were still a few potentially-slow bits in nm_manager_new() which are now moved into nm_manager_start().) --- src/main.c | 25 ++++++++++++------------- src/nm-manager.c | 18 +++++++++--------- src/settings/nm-settings.c | 30 +++++++++++++++++++++++------- src/settings/nm-settings.h | 3 ++- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/main.c b/src/main.c index 3ea05bebf8..3ba6518d2a 100644 --- a/src/main.c +++ b/src/main.c @@ -419,6 +419,16 @@ main (int argc, char *argv[]) #endif ); + nm_auth_manager_setup (nm_config_get_auth_polkit (config)); + + settings = nm_settings_new (); + manager = nm_manager_new (settings, + global_opt.state_file, + net_enabled, + wifi_enabled, + wwan_enabled, + wimax_enabled); + if (!nm_bus_manager_get_connection (nm_bus_manager_get ())) { #if HAVE_DBUS_GLIB_100 nm_log_warn (LOGD_CORE, "Failed to connect to D-Bus; only private bus is available"); @@ -443,24 +453,13 @@ main (int argc, char *argv[]) * NMPlatform. */ g_object_ref (NM_PLATFORM_GET); - nm_auth_manager_setup (nm_config_get_auth_polkit (config)); - nm_dispatcher_init (); - settings = nm_settings_new (&error); - if (!settings) { - nm_log_err (LOGD_CORE, "failed to initialize settings storage: %s", - error && error->message ? error->message : "(unknown)"); + if (!nm_settings_start (settings, &error)) { + nm_log_err (LOGD_CORE, "failed to initialize settings storage: %s", error->message); goto done; } - manager = nm_manager_new (settings, - global_opt.state_file, - net_enabled, - wifi_enabled, - wwan_enabled, - wimax_enabled); - g_signal_connect (manager, NM_MANAGER_CONFIGURE_QUIT, G_CALLBACK (manager_configure_quit), config); nm_manager_start (manager); diff --git a/src/nm-manager.c b/src/nm-manager.c index 3bda450930..3c5969aa40 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4198,6 +4198,11 @@ nm_manager_start (NMManager *self) NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); guint i; + g_signal_connect (nm_platform_get (), + NM_PLATFORM_SIGNAL_LINK_CHANGED, + G_CALLBACK (platform_link_cb), + self); + /* Set initial radio enabled/disabled state */ for (i = 0; i < RFKILL_TYPE_MAX; i++) { RadioState *rstate = &priv->radio_states[i]; @@ -4227,10 +4232,14 @@ nm_manager_start (NMManager *self) system_hostname_changed_cb (priv->settings, NULL, self); /* Start device factories */ + nm_device_factory_manager_load_factories (_register_device_factory, self); nm_device_factory_manager_for_each_factory (start_factory, NULL); platform_query_devices (self); + /* Load VPN plugins */ + priv->vpn_manager = g_object_ref (nm_vpn_manager_get ()); + /* * Connections added before the manager is started do not emit * connection-added signals thus devices have to be created manually. @@ -4831,11 +4840,6 @@ nm_manager_new (NMSettings *settings, nm_exported_object_export (NM_EXPORTED_OBJECT (singleton)); - g_signal_connect (nm_platform_get (), - NM_PLATFORM_SIGNAL_LINK_CHANGED, - G_CALLBACK (platform_link_cb), - singleton); - priv->rfkill_mgr = nm_rfkill_manager_new (); g_signal_connect (priv->rfkill_mgr, "rfkill-changed", @@ -4850,8 +4854,6 @@ nm_manager_new (NMSettings *settings, rfkill_change (priv->radio_states[RFKILL_TYPE_WLAN].desc, RFKILL_TYPE_WLAN, initial_wifi_enabled); rfkill_change (priv->radio_states[RFKILL_TYPE_WWAN].desc, RFKILL_TYPE_WWAN, initial_wwan_enabled); - nm_device_factory_manager_load_factories (_register_device_factory, singleton); - return singleton; } @@ -4899,8 +4901,6 @@ nm_manager_init (NMManager *manager) G_CALLBACK (dbus_connection_changed_cb), manager); - priv->vpn_manager = g_object_ref (nm_vpn_manager_get ()); - /* sleep/wake handling */ priv->sleep_monitor = g_object_ref (nm_sleep_monitor_get ()); g_signal_connect (priv->sleep_monitor, NM_SLEEP_MONITOR_SLEEPING, diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index d7b76c9cf6..0988e1cc0e 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -176,6 +176,7 @@ typedef struct { GSList *unrecognized_specs; GSList *get_connections_cache; + gboolean started; gboolean startup_complete; struct { @@ -561,6 +562,9 @@ nm_settings_get_hostname (NMSettings *self) NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); char *hostname = NULL; + if (!priv->started) + return NULL; + if (priv->hostname.hostnamed_proxy) { hostname = g_strdup (priv->hostname.value); goto out; @@ -2094,13 +2098,10 @@ setup_hostname_file_monitors (NMSettings *self) } NMSettings * -nm_settings_new (GError **error) +nm_settings_new (void) { NMSettings *self; NMSettingsPrivate *priv; - GDBusProxy *proxy; - GVariant *variant; - GError *local_error = NULL; self = g_object_new (NM_TYPE_SETTINGS, NULL); @@ -2109,10 +2110,24 @@ nm_settings_new (GError **error) priv->config = nm_config_get (); priv->dbus_mgr = nm_bus_manager_get (); + nm_exported_object_export (NM_EXPORTED_OBJECT (self)); + return self; +} + +gboolean +nm_settings_start (NMSettings *self, GError **error) +{ + NMSettingsPrivate *priv; + GDBusProxy *proxy; + GVariant *variant; + GError *local_error = NULL; + + priv = NM_SETTINGS_GET_PRIVATE (self); + /* Load the plugins; fail if a plugin is not found. */ if (!load_plugins (self, nm_config_get_plugins (priv->config), error)) { g_object_unref (self); - return NULL; + return FALSE; } load_connections (self); @@ -2143,8 +2158,9 @@ nm_settings_new (GError **error) if (!priv->hostname.hostnamed_proxy) setup_hostname_file_monitors (self); - nm_exported_object_export (NM_EXPORTED_OBJECT (self)); - return self; + priv->started = TRUE; + g_object_notify (G_OBJECT (self), NM_SETTINGS_HOSTNAME); + return TRUE; } static void diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h index 4afa4dc250..8e2d328952 100644 --- a/src/settings/nm-settings.h +++ b/src/settings/nm-settings.h @@ -73,7 +73,8 @@ typedef struct { GType nm_settings_get_type (void); -NMSettings *nm_settings_new (GError **error); +NMSettings *nm_settings_new (void); +gboolean nm_settings_start (NMSettings *self, GError **error); typedef void (*NMSettingsForEachFunc) (NMSettings *settings, NMSettingsConnection *connection,