clients/secret-agent: fix cancel_get_secrets() implementation

The callback must be invoked, as also documented.

Otherwise, the tracked info gets leaked.

Let NMSecretAgentOld (the caller) be a bit resilient against
bugs in the client, and avoid a crash by prematurely remove
the request-info from the pending list. That does not fully
workaround the bug (it leads to a leak), but at least it does
not cause other "severe" issues.

The leak was present earlier as well.

2 files changed, 20 insertions, 9 deletions

diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c
index 184f89e274..5a36dffa7a 100644
--- a/clients/common/nm-secret-agent-simple.c
+++ b/clients/common/nm-secret-agent-simple.c
@@ -1035,10 +1035,23 @@ cancel_get_secrets (NMSecretAgentOld *agent,
 {
 	NMSecretAgentSimple *self = NM_SECRET_AGENT_SIMPLE (agent);
 	NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self);
+	gs_free_error GError *error = NULL;
 	gs_free char *request_id = NULL;
+	RequestData *request;
 
 	request_id = g_strdup_printf ("%s/%s", connection_path, setting_name);
-	g_hash_table_remove (priv->requests, &request_id);
+	request = g_hash_table_lookup (priv->requests, &request_id);
+	if (!request) {
+		/* this is really a bug of the caller (or us?). We cannot invoke a callback,
+		 * hence the caller cannot cleanup the request. */
+		g_return_if_reached ();
+	}
+
+	g_set_error (&error,
+	             NM_SECRET_AGENT_ERROR,
+	             NM_SECRET_AGENT_ERROR_AGENT_CANCELED,
+	             "The secret agent is going away");
+	_request_data_complete (request, NULL, error, NULL);
 }
 
 static void
diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c
index 148d7c34cd..de80a46c0c 100644
--- a/libnm/nm-secret-agent-old.c
+++ b/libnm/nm-secret-agent-old.c
@@ -125,7 +125,6 @@ name_owner_changed (GObject *proxy,
 {
 	NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (user_data);
 	NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self);
-	GSList *iter;
 	char *owner;
 
 	owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (proxy));
@@ -134,16 +133,14 @@ name_owner_changed (GObject *proxy,
 			nm_secret_agent_old_register_async (self, NULL, NULL, NULL);
 		g_free (owner);
 	} else {
-		/* Cancel any pending secrets requests */
-		for (iter = priv->pending_gets; iter; iter = g_slist_next (iter)) {
-			GetSecretsInfo *info = iter->data;
+		while (priv->pending_gets) {
+			GetSecretsInfo *info = priv->pending_gets->data;
 
+			priv->pending_gets = g_slist_remove (priv->pending_gets, info);
 			NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self,
 			                                                          info->path,
 			                                                          info->setting_name);
 		}
-		g_slist_free (priv->pending_gets);
-		priv->pending_gets = NULL;
 
 		_internal_unregister (self);
 	}
@@ -307,7 +304,6 @@ get_secrets_cb (NMSecretAgentOld *self,
 		                                       g_variant_new ("(@a{sa{sv}})", secrets));
 	}
 
-	/* Remove the request from internal tracking */
 	get_secrets_info_finalize (self, info);
 }
 
@@ -390,10 +386,12 @@ impl_secret_agent_old_cancel_get_secrets (NMSecretAgentOld *self,
 		return;
 	}
 
-	/* Send the cancel request up to the subclass and finalize it */
+	priv->pending_gets = g_slist_remove (priv->pending_gets, info);
+
 	NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self,
 	                                                          info->path,
 	                                                          info->setting_name);
+
 	g_dbus_method_invocation_return_value (context, NULL);
 }