From 4d2da18f7d3fa79aeededcd1760d3a1ab75adfeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 1 Jun 2018 17:07:41 +0200 Subject: libgdm: Unref the manager propagated from task This instance has already been reffed when passed to the task, and since we're stealing it with `g_task_propagate_pointer` it won't be unreffed. We could also do this in the `on_reauthentication_channel_opened` callback but since the new task will ref it anyway, we can just be clean and do it here. --- libgdm/gdm-client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index f327344e..fa4ba842 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -390,7 +390,7 @@ on_got_manager_for_reauthentication (GdmClient *client, (GAsyncReadyCallback) on_reauthentication_channel_opened, task); - + g_object_unref (manager); } static GDBusConnection * @@ -527,6 +527,8 @@ on_got_manager_for_opening_connection (GdmClient *client, (GAsyncReadyCallback) on_session_opened, task); + + g_object_unref (manager); } static GDBusConnection * -- cgit v1.2.1 From d3b1cc17bb93de7025b2204c09545363f6f15264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 1 Jun 2018 17:16:35 +0200 Subject: libgdm: Don't double-ref the connection got from task Both if we re-use the shared connection in `gdm_client_get_connection` and if we create a new one in `on_connected`, we steal the pointer here by using `g_task_propagate_pointer` and thus we don't have to add an additional reference to this connection when returning, or it won't ever be consumed by function customers. --- libgdm/gdm-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index fa4ba842..36292148 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -536,7 +536,7 @@ gdm_client_get_connection_finish (GdmClient *client, GAsyncResult *result, GError **error) { - GDBusConnection *connection = NULL; + GDBusConnection *connection; g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); @@ -551,7 +551,7 @@ gdm_client_get_connection_finish (GdmClient *client, (gpointer *) &client->priv->connection); } - return g_object_ref (connection); + return connection; } static void -- cgit v1.2.1 From 1f5678c65c96d2534a687a2344fdc0655a832c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 1 Jun 2018 17:20:17 +0200 Subject: libgdm: Don't leak connection on sync re-authentication --- libgdm/gdm-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index 36292148..335a040a 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -604,7 +604,7 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, GCancellable *cancellable, GError **error) { - GDBusConnection *connection; + g_autoptr(GDBusConnection) connection = NULL; g_autoptr(GdmManager) manager = NULL; GdmUserVerifier *user_verifier = NULL; gboolean ret; -- cgit v1.2.1 From d92f4d93ce2d5d25db5777459fcd51e2d80645a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 1 Jun 2018 17:22:20 +0200 Subject: libgdm: Use auto-pointers and cleanup code --- libgdm/gdm-client.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index 335a040a..7b42b6c9 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -606,9 +606,9 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, { g_autoptr(GDBusConnection) connection = NULL; g_autoptr(GdmManager) manager = NULL; + g_autofree char *address = NULL; GdmUserVerifier *user_verifier = NULL; gboolean ret; - char *address; g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); @@ -620,7 +620,7 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, error); if (manager == NULL) { - goto out; + return NULL; } ret = gdm_manager_call_open_reauthentication_channel_sync (manager, @@ -630,10 +630,10 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, error); if (!ret) { - goto out; + return NULL; } - g_debug ("GdmClient: connecting to address: %s", client->priv->address); + g_debug ("GdmClient: connecting to address: %s", address); connection = g_dbus_connection_new_for_address_sync (address, G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, @@ -642,10 +642,8 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, error); if (connection == NULL) { - g_free (address); - goto out; + return NULL; } - g_free (address); user_verifier = gdm_user_verifier_proxy_new_sync (connection, G_DBUS_PROXY_FLAGS_NONE, @@ -654,7 +652,6 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, cancellable, error); - out: return user_verifier; } -- cgit v1.2.1 From 7782778781d0718882c005f94aed9f831e17aa48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sat, 2 Jun 2018 19:34:08 +0200 Subject: libgdb: Try to reuse connections from the available proxies Instead of using the hard-to-maintain shared pointer to the dbus connection to the manager and reset it when the proxies that use it are deleted, just look which proxy is currently available and try to reuse the connection from it. Fixes #386 --- libgdm/gdm-client.c | 70 +++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index 7b42b6c9..acf016a4 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -46,7 +46,6 @@ struct GdmClientPrivate GdmGreeter *greeter; GdmRemoteGreeter *remote_greeter; GdmChooser *chooser; - GDBusConnection *connection; char *address; char **enabled_extensions; @@ -71,6 +70,28 @@ gdm_client_error_quark (void) return error_quark; } +static GDBusConnection * +gdm_client_get_open_connection (GdmClient *client) +{ + GDBusProxy *proxy = NULL; + + if (client->priv->user_verifier != NULL) { + proxy = G_DBUS_PROXY (client->priv->user_verifier); + } else if (client->priv->greeter != NULL) { + proxy = G_DBUS_PROXY (client->priv->greeter); + } else if (client->priv->remote_greeter != NULL) { + proxy = G_DBUS_PROXY (client->priv->remote_greeter); + } else if (client->priv->chooser != NULL) { + proxy = G_DBUS_PROXY (client->priv->chooser); + } + + if (proxy != NULL) { + return g_dbus_proxy_get_connection (proxy); + } + + return NULL; +} + static void on_got_manager (GObject *object, GAsyncResult *result, @@ -399,12 +420,15 @@ gdm_client_get_connection_sync (GdmClient *client, GError **error) { g_autoptr(GdmManager) manager = NULL; + GDBusConnection *connection; gboolean ret; g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); - if (client->priv->connection != NULL) { - return g_object_ref (client->priv->connection); + connection = gdm_client_get_open_connection (client); + + if (connection != NULL) { + return g_object_ref (connection); } manager = gdm_manager_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, @@ -429,23 +453,19 @@ gdm_client_get_connection_sync (GdmClient *client, g_debug ("GdmClient: connecting to address: %s", client->priv->address); - client->priv->connection = g_dbus_connection_new_for_address_sync (client->priv->address, - G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, - NULL, - cancellable, - error); + connection = g_dbus_connection_new_for_address_sync (client->priv->address, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + NULL, + cancellable, + error); - if (client->priv->connection == NULL) { + if (connection == NULL) { g_clear_pointer (&client->priv->address, g_free); goto out; } - g_object_add_weak_pointer (G_OBJECT (client->priv->connection), - (gpointer *) - &client->priv->connection); - out: - return client->priv->connection; + return connection; } static void @@ -545,12 +565,6 @@ gdm_client_get_connection_finish (GdmClient *client, return NULL; } - if (client->priv->connection == NULL) { - client->priv->connection = connection; - g_object_add_weak_pointer (G_OBJECT (client->priv->connection), - (gpointer *) &client->priv->connection); - } - return connection; } @@ -561,6 +575,7 @@ gdm_client_get_connection (GdmClient *client, gpointer user_data) { GTask *task; + GDBusConnection *connection; g_return_if_fail (GDM_IS_CLIENT (client)); @@ -569,9 +584,10 @@ gdm_client_get_connection (GdmClient *client, callback, user_data); - if (client->priv->connection != NULL) { + connection = gdm_client_get_open_connection (client); + if (connection != NULL) { g_task_return_pointer (task, - g_object_ref (client->priv->connection), + g_object_ref (connection), (GDestroyNotify) g_object_unref); g_object_unref (task); return; @@ -775,7 +791,7 @@ gdm_client_get_user_verifier_sync (GdmClient *client, if (strcmp (client->priv->enabled_extensions[i], gdm_user_verifier_choice_list_interface_info ()->name) == 0) { GdmUserVerifierChoiceList *choice_list_interface; - choice_list_interface = gdm_user_verifier_choice_list_proxy_new_sync (client->priv->connection, + choice_list_interface = gdm_user_verifier_choice_list_proxy_new_sync (connection, G_DBUS_PROXY_FLAGS_NONE, NULL, SESSION_DBUS_PATH, @@ -1509,14 +1525,6 @@ gdm_client_finalize (GObject *object) &client->priv->chooser); } - if (client->priv->connection != NULL) { - g_object_remove_weak_pointer (G_OBJECT (client->priv->connection), - (gpointer *) - &client->priv->connection); - } - - g_clear_object (&client->priv->connection); - g_strfreev (client->priv->enabled_extensions); g_free (client->priv->address); -- cgit v1.2.1 From a01fc3ca1a9c1321f8782547d9487838007091e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sat, 2 Jun 2018 19:44:24 +0200 Subject: libgdm: Don't save manager address There's no need to keep the manager connection address around, and use autofree to clean it up --- libgdm/gdm-client.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index acf016a4..0e8bf439 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -46,7 +46,6 @@ struct GdmClientPrivate GdmGreeter *greeter; GdmRemoteGreeter *remote_greeter; GdmChooser *chooser; - char *address; char **enabled_extensions; }; @@ -420,6 +419,7 @@ gdm_client_get_connection_sync (GdmClient *client, GError **error) { g_autoptr(GdmManager) manager = NULL; + g_autofree char *address = NULL; GDBusConnection *connection; gboolean ret; @@ -439,32 +439,26 @@ gdm_client_get_connection_sync (GdmClient *client, error); if (manager == NULL) { - goto out; + return NULL; } ret = gdm_manager_call_open_session_sync (manager, - &client->priv->address, + &address, cancellable, error); if (!ret) { - goto out; + return NULL; } - g_debug ("GdmClient: connecting to address: %s", client->priv->address); + g_debug ("GdmClient: connecting to address: %s", address); - connection = g_dbus_connection_new_for_address_sync (client->priv->address, + connection = g_dbus_connection_new_for_address_sync (address, G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, NULL, cancellable, error); - if (connection == NULL) { - g_clear_pointer (&client->priv->address, g_free); - goto out; - } - - out: return connection; } @@ -497,6 +491,7 @@ on_session_opened (GdmManager *manager, GTask *task) { GdmClient *client; + g_autofree char *address = NULL; GCancellable *cancellable; GError *error; @@ -504,7 +499,7 @@ on_session_opened (GdmManager *manager, error = NULL; if (!gdm_manager_call_open_session_finish (manager, - &client->priv->address, + &address, result, &error)) { g_task_return_error (task, error); @@ -514,7 +509,7 @@ on_session_opened (GdmManager *manager, } cancellable = g_task_get_cancellable (task); - g_dbus_connection_new_for_address (client->priv->address, + g_dbus_connection_new_for_address (address, G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, NULL, cancellable, @@ -1526,7 +1521,6 @@ gdm_client_finalize (GObject *object) } g_strfreev (client->priv->enabled_extensions); - g_free (client->priv->address); G_OBJECT_CLASS (gdm_client_parent_class)->finalize (object); } -- cgit v1.2.1 From aca3d13d452ecf1a8e1e68ce955c8117bb99de7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 4 Jun 2018 19:13:04 +0200 Subject: libgdm: Return NULL on invalid client instances --- libgdm/gdm-client.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libgdm/gdm-client.c b/libgdm/gdm-client.c index 0e8bf439..28cb7253 100644 --- a/libgdm/gdm-client.c +++ b/libgdm/gdm-client.c @@ -423,7 +423,7 @@ gdm_client_get_connection_sync (GdmClient *client, GDBusConnection *connection; gboolean ret; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); connection = gdm_client_get_open_connection (client); @@ -553,7 +553,7 @@ gdm_client_get_connection_finish (GdmClient *client, { GDBusConnection *connection; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); connection = g_task_propagate_pointer (G_TASK (result), error); if (connection == NULL) { @@ -621,7 +621,7 @@ gdm_client_open_reauthentication_channel_sync (GdmClient *client, GdmUserVerifier *user_verifier = NULL; gboolean ret; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); manager = gdm_manager_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, @@ -722,7 +722,7 @@ gdm_client_open_reauthentication_channel_finish (GdmClient *client, GAsyncResult *result, GError **error) { - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); return g_task_propagate_pointer (G_TASK (result), error); } @@ -890,7 +890,7 @@ gdm_client_get_user_verifier_finish (GdmClient *client, { GdmUserVerifier *user_verifier; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); if (client->priv->user_verifier != NULL) return g_object_ref (client->priv->user_verifier); @@ -1058,7 +1058,7 @@ gdm_client_get_greeter_finish (GdmClient *client, { GdmGreeter *greeter; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); if (client->priv->greeter != NULL) return g_object_ref (client->priv->greeter); @@ -1232,7 +1232,7 @@ gdm_client_get_remote_greeter_finish (GdmClient *client, { GdmRemoteGreeter *remote_greeter; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); if (client->priv->remote_greeter != NULL) return g_object_ref (client->priv->remote_greeter); @@ -1403,7 +1403,7 @@ gdm_client_get_chooser_finish (GdmClient *client, { GdmChooser *chooser; - g_return_val_if_fail (GDM_IS_CLIENT (client), FALSE); + g_return_val_if_fail (GDM_IS_CLIENT (client), NULL); if (client->priv->chooser != NULL) return g_object_ref (client->priv->chooser); -- cgit v1.2.1