From f9ee20a7b2bd364384af2487da357344fd61a6b9 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 23 Jul 2015 10:03:21 -0500 Subject: [PATCH] core: explicitly unexport objects when we're done with them Previously most objects were implicitly unexported when they were destroyed, but since refcounts may make the object live longer than intended, we should explicitly unexport them when they should no longer be present on the bus. This means we can assume that objects will always be un-exported already when they are destroyed, *except* when quitting where most objects will live until exit because NM leaves interfaces up and running on quit. --- src/devices/adsl/nm-device-adsl.c | 8 ++--- src/devices/nm-device-ethernet.c | 8 ++--- src/devices/nm-device.c | 17 +++++------ src/devices/wwan/nm-modem.c | 8 ++--- src/main.c | 3 ++ src/nm-exported-object.c | 43 +++++++++++++++++++++++++-- src/nm-exported-object.h | 5 ++++ src/nm-manager.c | 5 ++-- src/settings/nm-settings-connection.c | 7 ----- src/settings/nm-settings.c | 2 ++ src/vpn-manager/nm-vpn-connection.c | 16 +++++----- 11 files changed, 75 insertions(+), 47 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 019cccfa59..08a7511abe 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -441,8 +441,7 @@ act_stage3_ip4_config_start (NMDevice *device, _LOGW (LOGD_ADSL, "PPP failed to start: %s", err->message); g_error_free (err); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; } @@ -456,10 +455,7 @@ deactivate (NMDevice *device) NMDeviceAdsl *self = NM_DEVICE_ADSL (device); NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); g_signal_handlers_disconnect_by_func (nm_platform_get (), G_CALLBACK (link_changed_cb), device); diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index d1202dc72f..560057ad38 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -1010,8 +1010,7 @@ pppoe_stage3_ip4_config_start (NMDeviceEthernet *self, NMDeviceStateReason *reas _LOGW (LOGD_DEVICE, "PPPoE failed to start: %s", err->message); g_error_free (err); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; } @@ -1399,10 +1398,7 @@ deactivate (NMDevice *device) priv->pending_ip4_config = NULL; } - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); supplicant_interface_release (self); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 89b7854dbc..9e7243705d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3581,7 +3581,7 @@ dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) } if (priv->dhcp4_config) { - g_clear_object (&priv->dhcp4_config); + nm_exported_object_clear_and_unexport (&priv->dhcp4_config); g_object_notify (G_OBJECT (self), NM_DEVICE_DHCP4_CONFIG); } } @@ -3938,8 +3938,7 @@ dhcp4_start (NMDevice *self, s_ip4 = nm_connection_get_setting_ip4_config (connection); /* Clear old exported DHCP options */ - if (priv->dhcp4_config) - g_object_unref (priv->dhcp4_config); + nm_exported_object_clear_and_unexport (&priv->dhcp4_config); priv->dhcp4_config = nm_dhcp4_config_new (); hw_addr = nm_platform_link_get_address (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self), &hw_addr_len); @@ -4275,7 +4274,7 @@ dhcp6_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) nm_device_remove_pending_action (self, PENDING_ACTION_DHCP6, FALSE); if (priv->dhcp6_config) { - g_clear_object (&priv->dhcp6_config); + nm_exported_object_clear_and_unexport (&priv->dhcp6_config); g_object_notify (G_OBJECT (self), NM_DEVICE_DHCP6_CONFIG); } } @@ -4700,7 +4699,7 @@ dhcp6_start (NMDevice *self, gboolean wait_for_ll, NMDeviceStateReason *reason) NMConnection *connection; NMSettingIPConfig *s_ip6; - g_clear_object (&priv->dhcp6_config); + nm_exported_object_clear_and_unexport (&priv->dhcp6_config); priv->dhcp6_config = nm_dhcp6_config_new (); g_warn_if_fail (priv->dhcp6_ip6_config == NULL); @@ -6864,8 +6863,8 @@ nm_device_set_ip4_config (NMDevice *self, g_object_notify (G_OBJECT (self), NM_DEVICE_IP4_CONFIG); g_signal_emit (self, signals[IP4_CONFIG_CHANGED], 0, priv->ip4_config, old_config); - if (old_config != priv->ip4_config && old_config) - g_object_unref (old_config); + if (old_config != priv->ip4_config) + nm_exported_object_clear_and_unexport (&old_config); if (nm_device_uses_generated_assumed_connection (self)) { NMConnection *connection = nm_device_get_applied_connection (self); @@ -7031,8 +7030,8 @@ nm_device_set_ip6_config (NMDevice *self, g_object_notify (G_OBJECT (self), NM_DEVICE_IP6_CONFIG); g_signal_emit (self, signals[IP6_CONFIG_CHANGED], 0, priv->ip6_config, old_config); - if (old_config != priv->ip6_config && old_config) - g_object_unref (old_config); + if (old_config != priv->ip6_config) + nm_exported_object_clear_and_unexport (&old_config); if (nm_device_uses_generated_assumed_connection (self)) { NMConnection *connection = nm_device_get_applied_connection (self); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 7e70ba5608..5889aa7805 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -530,8 +530,7 @@ ppp_stage3_ip_config_start (NMModem *self, error && error->message ? error->message : "(unknown)"); g_error_free (error); - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; + nm_exported_object_clear_and_unexport (&priv->ppp_manager); *reason = NM_DEVICE_STATE_REASON_PPP_START_FAILED; ret = NM_ACT_STAGE_RETURN_FAILURE; @@ -885,10 +884,7 @@ deactivate_cleanup (NMModem *self, NMDevice *device) priv->in_bytes = priv->out_bytes = 0; - if (priv->ppp_manager) { - g_object_unref (priv->ppp_manager); - priv->ppp_manager = NULL; - } + nm_exported_object_clear_and_unexport (&priv->ppp_manager); if (device) { g_return_if_fail (NM_IS_DEVICE (device)); diff --git a/src/main.c b/src/main.c index 7e36a3821c..a0c5c048b7 100644 --- a/src/main.c +++ b/src/main.c @@ -50,6 +50,7 @@ #include "nm-settings.h" #include "nm-auth-manager.h" #include "nm-core-internal.h" +#include "nm-exported-object.h" #if !defined(NM_DIST_VERSION) # define NM_DIST_VERSION VERSION @@ -454,6 +455,8 @@ main (int argc, char *argv[]) if (configure_and_quit == FALSE) g_main_loop_run (main_loop); + nm_exported_object_class_set_quitting (); + nm_manager_stop (nm_manager_get ()); done: diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index 845eaa4190..3a22b5c24b 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -28,6 +28,8 @@ #include "nm-default.h" static GHashTable *prefix_counters; +static gboolean quitting = FALSE; + #if NM_MORE_ASSERTS >= 2 #define _ASSERT_NO_EARLY_EXPORT @@ -632,6 +634,28 @@ nm_exported_object_get_interface_by_type (NMExportedObject *self, GType interfac return NULL; } +void +_nm_exported_object_clear_and_unexport (NMExportedObject **location) +{ + NMExportedObject *self; + NMExportedObjectPrivate *priv; + + if (!location || !*location) + return; + + self = *location; + *location = NULL; + + g_return_if_fail (NM_IS_EXPORTED_OBJECT (self)); + + priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); + + if (priv->path) + nm_exported_object_unexport (self); + + g_object_unref (self); +} + static void nm_exported_object_init (NMExportedObject *self) { @@ -765,8 +789,16 @@ nm_exported_object_dispose (GObject *object) { NMExportedObjectPrivate *priv = NM_EXPORTED_OBJECT_GET_PRIVATE (object); - if (priv->path) - nm_exported_object_unexport (NM_EXPORTED_OBJECT (object)); + /* Objects should have already been unexported by their owner, unless + * we are quitting, where many objects stick around until exit. + */ + if (!quitting) { + if (priv->path) { + g_warn_if_reached (); + nm_exported_object_unexport (NM_EXPORTED_OBJECT (object)); + } + } else + g_clear_pointer (&priv->path, g_free); g_variant_builder_clear (&priv->pending_notifies); nm_clear_g_source (&priv->notify_idle_id); @@ -785,3 +817,10 @@ nm_exported_object_class_init (NMExportedObjectClass *klass) object_class->notify = nm_exported_object_notify; object_class->dispose = nm_exported_object_dispose; } + +void +nm_exported_object_class_set_quitting (void) +{ + quitting = TRUE; +} + diff --git a/src/nm-exported-object.h b/src/nm-exported-object.h index 0b952b837a..cf2cacd1cb 100644 --- a/src/nm-exported-object.h +++ b/src/nm-exported-object.h @@ -45,6 +45,8 @@ typedef struct { GType nm_exported_object_get_type (void); +void nm_exported_object_class_set_quitting (void); + void nm_exported_object_class_add_interface (NMExportedObjectClass *object_class, GType dbus_skeleton_type, ...) G_GNUC_NULL_TERMINATED; @@ -56,6 +58,9 @@ void nm_exported_object_unexport (NMExportedObject *self); GSList * nm_exported_object_get_interfaces (NMExportedObject *self); GDBusInterfaceSkeleton *nm_exported_object_get_interface_by_type (NMExportedObject *self, GType interface_type); +void _nm_exported_object_clear_and_unexport (NMExportedObject **location); +#define nm_exported_object_clear_and_unexport(location) _nm_exported_object_clear_and_unexport ((NMExportedObject **) (location)) + G_END_DECLS #endif /* NM_EXPORTED_OBJECT_H */ diff --git a/src/nm-manager.c b/src/nm-manager.c index a3d4117008..5cba730fec 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -225,7 +225,7 @@ active_connection_remove (NMManager *self, NMActiveConnection *active) else connection = NULL; - g_object_unref (active); + nm_exported_object_clear_and_unexport (&active); if ( connection && nm_settings_has_connection (priv->settings, connection)) { @@ -801,8 +801,7 @@ remove_device (NMManager *manager, g_object_notify (G_OBJECT (manager), NM_MANAGER_DEVICES); nm_device_removed (device); - nm_exported_object_unexport (NM_EXPORTED_OBJECT (device)); - g_object_unref (device); + nm_exported_object_clear_and_unexport (&device); check_if_startup_complete (manager); } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 29162c7984..20f4578da3 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2053,14 +2053,7 @@ nm_settings_connection_signal_remove (NMSettingsConnection *self) if (priv->removed) g_return_if_reached (); priv->removed = TRUE; - - /* Emit removed first */ g_signal_emit_by_name (self, NM_SETTINGS_CONNECTION_REMOVED); - - /* And unregister last to ensure the removed signal goes out before - * we take the connection off the bus. - */ - nm_exported_object_unexport (NM_EXPORTED_OBJECT (self)); } gboolean diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 951f417646..bdea933249 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -905,6 +905,8 @@ connection_removed (NMSettingsConnection *connection, gpointer user_data) /* Re-emit for listeners like NMPolicy */ g_signal_emit_by_name (self, NM_CP_SIGNAL_CONNECTION_REMOVED, connection); g_object_notify (G_OBJECT (self), NM_SETTINGS_CONNECTIONS); + if (nm_exported_object_is_exported (NM_EXPORTED_OBJECT (connection))) + nm_exported_object_unexport (NM_EXPORTED_OBJECT (connection)); check_startup_complete (self); diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 0c0ac1c740..5f41050c11 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1117,8 +1117,8 @@ _cleanup_failed_config (NMVpnConnection *self) { NMVpnConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); - g_clear_object (&priv->ip4_config); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); _LOGW ("VPN connection: did not receive valid IP config information"); _set_vpn_state (self, STATE_FAILED, NM_VPN_CONNECTION_STATE_REASON_IP_CONFIG_INVALID, FALSE); @@ -1316,12 +1316,12 @@ nm_vpn_connection_config_get (NMVpnConnection *self, GVariant *dict) priv->has_ip4 = FALSE; if (g_variant_lookup (dict, NM_VPN_PLUGIN_CONFIG_HAS_IP4, "b", &b)) priv->has_ip4 = b; - g_clear_object (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); priv->has_ip6 = FALSE; if (g_variant_lookup (dict, NM_VPN_PLUGIN_CONFIG_HAS_IP6, "b", &b)) priv->has_ip6 = b; - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); } guint32 @@ -1491,7 +1491,7 @@ nm_vpn_connection_ip4_config_get (NMVpnConnection *self, GVariant *dict) nm_connection_get_setting_ip4_config (_get_applied_connection (self)), route_metric); - g_clear_object (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); priv->ip4_config = config; nm_exported_object_export (NM_EXPORTED_OBJECT (config)); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP4_CONFIG); @@ -1626,7 +1626,7 @@ next: nm_connection_get_setting_ip6_config (_get_applied_connection (self)), route_metric); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); priv->ip6_config = config; nm_exported_object_export (NM_EXPORTED_OBJECT (config)); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP6_CONFIG); @@ -2444,8 +2444,8 @@ dispose (GObject *object) g_cancellable_cancel (priv->cancellable); g_clear_object (&priv->cancellable); } - g_clear_object (&priv->ip4_config); - g_clear_object (&priv->ip6_config); + nm_exported_object_clear_and_unexport (&priv->ip4_config); + nm_exported_object_clear_and_unexport (&priv->ip6_config); g_clear_object (&priv->proxy); g_clear_object (&priv->plugin_info);