libnm-core: clear secrets from NMSimpleConnection and NMSettingsConnection dispose()

A few of the settings plugins were calling nm_connection_clear_secrets()
from their finalize() method, but this call can emit signals, and by
the time finalize() runs, the object has a refcount of 0.  Signals
cannot be emitted from a finalized object, but instead could be
emitted from dispose() before the object is finalized.

Instead of moving the nm_connection_clear_secrets() to dispose() in each
plugin, make the behavior generic instead.  The settings plugins' parent
object is NMSettingsConnection, so clear secrets there.  Plus,
NMSettingsConnection caches system & agent secrets with NMSimpleConnection
objects, so clear secrets in NMSimpleConnection's dispose too.

5 files changed, 41 insertions, 40 deletions

diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c
index 65592eb40a..bb398179de 100644
--- a/libnm-core/nm-simple-connection.c
+++ b/libnm-core/nm-simple-connection.c
@@ -100,8 +100,19 @@ nm_simple_connection_new_clone (NMConnection *connection)
 }
 
 static void
+dispose (GObject *object)
+{
+	nm_connection_clear_secrets (NM_CONNECTION (object));
+
+	G_OBJECT_CLASS (nm_simple_connection_parent_class)->dispose (object);
+}
+
+static void
 nm_simple_connection_class_init (NMSimpleConnectionClass *simple_class)
 {
+	GObjectClass *object_class = G_OBJECT_CLASS (simple_class);
+
+	object_class->dispose = dispose;
 }
 
 static void
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 8c9ea81247..d0a4f7ddb1 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -93,8 +93,6 @@ enum {
 static guint signals[LAST_SIGNAL] = { 0 };
 
 typedef struct {
-	gboolean disposed;
-
 	NMAgentManager *agent_mgr;
 	NMSessionMonitor *session_monitor;
 	guint session_changed_id;
@@ -2049,40 +2047,35 @@ dispose (GObject *object)
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
 	GSList *iter;
 
-	if (priv->disposed)
-		goto out;
-	priv->disposed = TRUE;
-
 	if (priv->updated_idle_id) {
 		g_source_remove (priv->updated_idle_id);
 		priv->updated_idle_id = 0;
 	}
 
-	if (priv->system_secrets)
-		g_object_unref (priv->system_secrets);
-	if (priv->agent_secrets)
-		g_object_unref (priv->agent_secrets);
+	nm_connection_clear_secrets (NM_CONNECTION (self));
+	g_clear_object (&priv->system_secrets);
+	g_clear_object (&priv->agent_secrets);
 
 	/* Cancel PolicyKit requests */
-	for (iter = priv->pending_auths; iter; iter = g_slist_next (iter))
-		nm_auth_chain_unref ((NMAuthChain *) iter->data);
-	g_slist_free (priv->pending_auths);
+	g_slist_free_full (priv->pending_auths, (GDestroyNotify) nm_auth_chain_unref);
 	priv->pending_auths = NULL;
 
 	/* Cancel in-progress secrets requests */
 	for (iter = priv->reqs; iter; iter = g_slist_next (iter))
 		nm_agent_manager_cancel_secrets (priv->agent_mgr, GPOINTER_TO_UINT (iter->data));
 	g_slist_free (priv->reqs);
+	priv->reqs = NULL;
 
-	g_hash_table_destroy (priv->seen_bssids);
+	g_clear_pointer (&priv->seen_bssids, (GDestroyNotify) g_hash_table_destroy);
 
 	set_visible (self, FALSE);
 
-	if (priv->session_changed_id)
+	if (priv->session_changed_id) {
 		g_signal_handler_disconnect (priv->session_monitor, priv->session_changed_id);
-	g_object_unref (priv->agent_mgr);
+		priv->session_changed_id = 0;
+	}
+	g_clear_object (&priv->agent_mgr);
 
-out:
 	G_OBJECT_CLASS (nm_settings_connection_parent_class)->dispose (object);
 }
 
diff --git a/src/settings/plugins/example/nm-example-connection.c b/src/settings/plugins/example/nm-example-connection.c
index 8cbeeeaaa9..c634388e9f 100644
--- a/src/settings/plugins/example/nm-example-connection.c
+++ b/src/settings/plugins/example/nm-example-connection.c
@@ -165,12 +165,7 @@ nm_example_connection_init (NMExampleConnection *connection)
 static void
 finalize (GObject *object)
 {
-	NMExampleConnectionPrivate *priv = NM_EXAMPLE_CONNECTION_GET_PRIVATE (object);
-
-	/* Zero out any secrets so we don't leave them in memory */
-	nm_connection_clear_secrets (NM_CONNECTION (object));
-
-	g_free (priv->path);
+	g_free (NM_EXAMPLE_CONNECTION_GET_PRIVATE (object)->path);
 
 	G_OBJECT_CLASS (nm_example_connection_parent_class)->finalize (object);
 }
diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
index ee936e5f85..463600959a 100644
--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
+++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
@@ -343,17 +343,6 @@ nm_ifcfg_connection_init (NMIfcfgConnection *connection)
 }
 
 static void
-finalize (GObject *object)
-{
-	nm_connection_clear_secrets (NM_CONNECTION (object));
-
-	path_watch_stop (NM_IFCFG_CONNECTION (object));
-	g_free (NM_IFCFG_CONNECTION_GET_PRIVATE (object)->path);
-
-	G_OBJECT_CLASS (nm_ifcfg_connection_parent_class)->finalize (object);
-}
-
-static void
 set_property (GObject *object, guint prop_id,
 		    const GValue *value, GParamSpec *pspec)
 {
@@ -392,6 +381,22 @@ get_property (GObject *object, guint prop_id,
 }
 
 static void
+dispose (GObject *object)
+{
+	path_watch_stop (NM_IFCFG_CONNECTION (object));
+
+	G_OBJECT_CLASS (nm_ifcfg_connection_parent_class)->dispose (object);
+}
+
+static void
+finalize (GObject *object)
+{
+	g_free (NM_IFCFG_CONNECTION_GET_PRIVATE (object)->path);
+
+	G_OBJECT_CLASS (nm_ifcfg_connection_parent_class)->finalize (object);
+}
+
+static void
 nm_ifcfg_connection_class_init (NMIfcfgConnectionClass *ifcfg_connection_class)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (ifcfg_connection_class);
@@ -402,6 +407,7 @@ nm_ifcfg_connection_class_init (NMIfcfgConnectionClass *ifcfg_connection_class)
 	/* Virtual methods */
 	object_class->set_property = set_property;
 	object_class->get_property = get_property;
+	object_class->dispose      = dispose;
 	object_class->finalize     = finalize;
 	settings_class->delete = do_delete;
 	settings_class->commit_changes = commit_changes;
diff --git a/src/settings/plugins/keyfile/nm-keyfile-connection.c b/src/settings/plugins/keyfile/nm-keyfile-connection.c
index 3b89607bc2..c078b6de09 100644
--- a/src/settings/plugins/keyfile/nm-keyfile-connection.c
+++ b/src/settings/plugins/keyfile/nm-keyfile-connection.c
@@ -166,11 +166,7 @@ nm_keyfile_connection_init (NMKeyfileConnection *connection)
 static void
 finalize (GObject *object)
 {
-	NMKeyfileConnectionPrivate *priv = NM_KEYFILE_CONNECTION_GET_PRIVATE (object);
-
-	nm_connection_clear_secrets (NM_CONNECTION (object));
-
-	g_free (priv->path);
+	g_free (NM_KEYFILE_CONNECTION_GET_PRIVATE (object)->path);
 
 	G_OBJECT_CLASS (nm_keyfile_connection_parent_class)->finalize (object);
 }
@@ -184,7 +180,7 @@ nm_keyfile_connection_class_init (NMKeyfileConnectionClass *keyfile_connection_c
 	g_type_class_add_private (keyfile_connection_class, sizeof (NMKeyfileConnectionPrivate));
 
 	/* Virtual methods */
-	object_class->finalize     = finalize;
+	object_class->finalize = finalize;
 	settings_class->commit_changes = commit_changes;
 	settings_class->delete = do_delete;
 }