From 9bf0b32cd1fcc12db2cd65ff28f2e1ddfd625d58 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 13 Feb 2018 14:39:31 +0100 Subject: [PATCH] cli/connections: avoid using synchronous get_secrets() With --ask it might call back to nmcli's agent, causing a deadlock while the client is waiting for the response. Let's give the client a chance to service the agent requests while waiting: $ nmcli --ask --show-secrets c show 'Oracle HQ' This is probably still rather suboptimal and inefficient, since we still serialize the calls and block on response. However, if we submit multiple calls to GetSecrets, the daemon would start authorizing the first one and fail the other ones immediately before the authorization succeeds. This could perhaps be addressed in the daemon, but let's settle for a fix that's compatible with the current daemon for now. --- clients/cli/connections.c | 52 ++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 519780a346..f08644db6f 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -610,26 +610,54 @@ get_ac_for_connection (const GPtrArray *active_cons, NMConnection *connection) return ac; } +typedef struct { + GMainLoop *loop; + NMConnection *local; + const char *setting_name; +} GetSecretsData; + +static void +got_secrets (GObject *source_object, GAsyncResult *res, gpointer user_data) +{ + NMRemoteConnection *remote = NM_REMOTE_CONNECTION (source_object); + GetSecretsData *data = user_data; + GVariant *secrets; + GError *error = NULL; + + secrets = nm_remote_connection_get_secrets_finish (remote, res, NULL); + if (secrets) { + if (!nm_connection_update_secrets (data->local, NULL, secrets, &error) && error) { + g_printerr (_("Error updating secrets for %s: %s\n"), + data->setting_name, error->message); + g_clear_error (&error); + } + g_variant_unref (secrets); + } + + g_main_loop_quit (data->loop); +} + /* Put secrets into local connection. */ static void update_secrets_in_connection (NMRemoteConnection *remote, NMConnection *local) { - GVariant *secrets; + GetSecretsData data = { 0, }; int i; - GError *error = NULL; + + data.local = local; + data.loop = g_main_loop_new (NULL, FALSE); for (i = 0; i < _NM_META_SETTING_TYPE_NUM; i++) { - secrets = nm_remote_connection_get_secrets (remote, nm_meta_setting_infos[i].setting_name, NULL, NULL); - if (secrets) { - if (!nm_connection_update_secrets (local, NULL, secrets, &error) && error) { - g_printerr (_("Error updating secrets for %s: %s\n"), - nm_meta_setting_infos[i].setting_name, - error->message); - g_clear_error (&error); - } - g_variant_unref (secrets); - } + data.setting_name = nm_meta_setting_infos[i].setting_name; + nm_remote_connection_get_secrets_async (remote, + nm_meta_setting_infos[i].setting_name, + NULL, + got_secrets, + &data); + g_main_loop_run (data.loop); } + + g_main_loop_unref (data.loop); } static gboolean