From 1a42e764d4376400f2db29319ce86f9570efc2ef Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 30 Aug 2013 17:57:56 -0500 Subject: [PATCH] core: fix handling of ActiveConnections on Policy dispose() The manager has already disposed of the ActiveConnections by the time the Policy is disposed, but the manager wasn't clearing the active_connections list, so the Policy got a stale list of freed objects. Next, the manager wasn't always emitting ACTIVE_CONNECTION_REMOVED when disposing of ActiveConnections, which the Policy listens to for cleanup. This lead to warnings on shutdown when the Policy attempted to clean up for already disposed objects Fix all this by ensuring the Manager signals when removing ActiveConnections, which the Policy then uses to clean up it's stuff, and ensuring the manager properly cleans up its ActiveConnection list. --- src/nm-manager.c | 19 ++++++++++++------- src/nm-policy.c | 7 +++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 2cefc8e887..ab9231b080 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -330,6 +330,14 @@ static void active_connection_state_changed (NMActiveConnection *active, GParamSpec *pspec, NMManager *self); +static void +active_connection_removed (NMManager *self, NMActiveConnection *active) +{ + g_signal_emit (self, signals[ACTIVE_CONNECTION_REMOVED], 0, active); + g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); + g_object_unref (active); +} + static gboolean _active_connection_cleanup (gpointer user_data) { @@ -347,9 +355,7 @@ _active_connection_cleanup (gpointer user_data) iter = iter->next; if (nm_active_connection_get_state (ac) == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { priv->active_connections = g_slist_remove (priv->active_connections, ac); - g_signal_emit (self, signals[ACTIVE_CONNECTION_REMOVED], 0, ac); - g_signal_handlers_disconnect_by_func (ac, active_connection_state_changed, self); - g_object_unref (ac); + active_connection_removed (self, ac); changed = TRUE; } } @@ -4554,11 +4560,10 @@ dispose (GObject *object) priv->ac_cleanup_id = 0; } - for (iter = priv->active_connections; iter; iter = g_slist_next (iter)) { - g_signal_handlers_disconnect_by_func (iter->data, active_connection_state_changed, object); - g_object_unref (iter->data); - } + for (iter = priv->active_connections; iter; iter = g_slist_next (iter)) + active_connection_removed (manager, NM_ACTIVE_CONNECTION (iter->data)); g_slist_free (priv->active_connections); + priv->active_connections = NULL; g_clear_object (&priv->primary_connection); g_clear_object (&priv->activating_connection); diff --git a/src/nm-policy.c b/src/nm-policy.c index 509343eeef..aca9d612b0 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2182,9 +2182,12 @@ dispose (GObject *object) } g_clear_pointer (&priv->dev_ids, g_slist_free); + /* The manager should have disposed of ActiveConnections already, which + * will have called active_connection_removed() and thus we don't need + * to clean anything up. Assert that this is TRUE. + */ connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) - active_connection_removed (priv->manager, NM_ACTIVE_CONNECTION (iter->data), policy); + g_assert (connections == NULL); if (priv->reset_retries_id) { g_source_remove (priv->reset_retries_id);