1From 56d371942e43c52bc6131067e2dc2a35f6cd5a3d Mon Sep 17 00:00:00 2001 2From: Philip Withnall <pwithnall@endlessos.org> 3Date: Mon, 13 Jun 2022 13:06:06 +0100 4Subject: [PATCH] gsocketclient: Fix still-reachable references to cancellables 5MIME-Version: 1.0 6Content-Type: text/plain; charset=UTF-8 7Content-Transfer-Encoding: 8bit 8 9`GSocketClient` chains its internal `GCancellable` objects to ones 10provided by the caller in two places using `g_cancellable_connect()`. 11However, it never calls `g_cancellable_disconnect()`, instead relying 12(incorrectly) on the `GCancellable` provided by the caller being 13short-lived. 14 15In the (valid) situation where a caller reuses one `GCancellable` for 16multiple socket client calls, or for calls across multiple socket 17clients, this will cause the internal `GCancellable` objects from those 18`GSocketClient`s to accumulate, with one reference left each (which is 19the reference from the `g_cancellable_connect()` closure). 20 21These `GCancellable` instances aren't technically leaked, as they will 22all be freed when the caller's `GCancellable` is disposed, but they are 23no longer useful and there is no bound on the number of them which will 24hang around. 25 26For a program doing a lot of socket operations, this still-reachable 27memory usage can become significant. 28 29Fix the problem by adding paired `g_cancellable_disconnect()` calls. 30It's not possible to add a unit test as we can't measure still-reachable 31memory growth before the end of a unit test when everything has to be 32freed. 33 34Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 35 36Fixes: #2670 37 38Conflict:NA 39Reference:https://gitlab.gnome.org/GNOME/glib/-/commit/56d371942e43c52bc6131067e2dc2a35f6cd5a3d 40 41--- 42 gio/gsocketclient.c | 23 +++++++++++++++++++---- 43 1 file changed, 19 insertions(+), 4 deletions(-) 44 45diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c 46index ae80f5203c..127915b722 100644 47--- a/gio/gsocketclient.c 48+++ b/gio/gsocketclient.c 49@@ -1466,6 +1466,8 @@ typedef struct 50 GSocketConnectable *connectable; 51 GSocketAddressEnumerator *enumerator; 52 GCancellable *enumeration_cancellable; 53+ GCancellable *enumeration_parent_cancellable; /* (nullable) (owned) */ 54+ gulong enumeration_cancelled_id; 55 56 GSList *connection_attempts; 57 GSList *successful_connections; 58@@ -1485,7 +1487,12 @@ g_socket_client_async_connect_data_free (GSocketClientAsyncConnectData *data) 59 data->task = NULL; 60 g_clear_object (&data->connectable); 61 g_clear_object (&data->enumerator); 62+ 63+ g_cancellable_disconnect (data->enumeration_parent_cancellable, data->enumeration_cancelled_id); 64+ g_clear_object (&data->enumeration_parent_cancellable); 65+ data->enumeration_cancelled_id = 0; 66 g_clear_object (&data->enumeration_cancellable); 67+ 68 g_slist_free_full (data->connection_attempts, connection_attempt_unref); 69 g_slist_free_full (data->successful_connections, connection_attempt_unref); 70 71@@ -1503,6 +1510,7 @@ typedef struct 72 GSocketClientAsyncConnectData *data; /* unowned */ 73 GSource *timeout_source; 74 GCancellable *cancellable; 75+ gulong cancelled_id; 76 grefcount ref; 77 } ConnectionAttempt; 78 79@@ -1530,6 +1538,8 @@ connection_attempt_unref (gpointer pointer) 80 g_clear_object (&attempt->address); 81 g_clear_object (&attempt->socket); 82 g_clear_object (&attempt->connection); 83+ g_cancellable_disconnect (g_task_get_cancellable (attempt->data->task), attempt->cancelled_id); 84+ attempt->cancelled_id = 0; 85 g_clear_object (&attempt->cancellable); 86 g_clear_object (&attempt->proxy_addr); 87 if (attempt->timeout_source) 88@@ -2023,8 +2033,9 @@ g_socket_client_enumerator_callback (GObject *object, 89 data->connection_attempts = g_slist_append (data->connection_attempts, attempt); 90 91 if (g_task_get_cancellable (data->task)) 92- g_cancellable_connect (g_task_get_cancellable (data->task), G_CALLBACK (on_connection_cancelled), 93- g_object_ref (attempt->cancellable), g_object_unref); 94+ attempt->cancelled_id = 95+ g_cancellable_connect (g_task_get_cancellable (data->task), G_CALLBACK (on_connection_cancelled), 96+ g_object_ref (attempt->cancellable), g_object_unref); 97 98 g_socket_connection_set_cached_remote_address ((GSocketConnection *)attempt->connection, address); 99 g_debug ("GSocketClient: Starting TCP connection attempt"); 100@@ -2129,8 +2140,12 @@ g_socket_client_connect_async (GSocketClient *client, 101 102 data->enumeration_cancellable = g_cancellable_new (); 103 if (cancellable) 104- g_cancellable_connect (cancellable, G_CALLBACK (on_connection_cancelled), 105- g_object_ref (data->enumeration_cancellable), g_object_unref); 106+ { 107+ data->enumeration_parent_cancellable = g_object_ref (cancellable); 108+ data->enumeration_cancelled_id = 109+ g_cancellable_connect (cancellable, G_CALLBACK (on_connection_cancelled), 110+ g_object_ref (data->enumeration_cancellable), g_object_unref); 111+ } 112 113 enumerator_next_async (data, FALSE); 114 } 115-- 116GitLab 117 118