From 56d371942e43c52bc6131067e2dc2a35f6cd5a3d Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@endlessos.org>
Date: Mon, 13 Jun 2022 13:06:06 +0100
Subject: [PATCH] gsocketclient: Fix still-reachable references to cancellables
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`GSocketClient` chains its internal `GCancellable` objects to ones
provided by the caller in two places using `g_cancellable_connect()`.
However, it never calls `g_cancellable_disconnect()`, instead relying
(incorrectly) on the `GCancellable` provided by the caller being
short-lived.

In the (valid) situation where a caller reuses one `GCancellable` for
multiple socket client calls, or for calls across multiple socket
clients, this will cause the internal `GCancellable` objects from those
`GSocketClient`s to accumulate, with one reference left each (which is
the reference from the `g_cancellable_connect()` closure).

These `GCancellable` instances aren't technically leaked, as they will
all be freed when the caller's `GCancellable` is disposed, but they are
no longer useful and there is no bound on the number of them which will
hang around.

For a program doing a lot of socket operations, this still-reachable
memory usage can become significant.

Fix the problem by adding paired `g_cancellable_disconnect()` calls.
It's not possible to add a unit test as we can't measure still-reachable
memory growth before the end of a unit test when everything has to be
freed.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2670

Conflict:NA
Reference:https://gitlab.gnome.org/GNOME/glib/-/commit/56d371942e43c52bc6131067e2dc2a35f6cd5a3d

---
 gio/gsocketclient.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c
index ae80f5203c..127915b722 100644
--- a/gio/gsocketclient.c
+++ b/gio/gsocketclient.c
@@ -1466,6 +1466,8 @@ typedef struct
   GSocketConnectable *connectable;
   GSocketAddressEnumerator *enumerator;
   GCancellable *enumeration_cancellable;
+  GCancellable *enumeration_parent_cancellable;  /* (nullable) (owned) */
+  gulong enumeration_cancelled_id;
 
   GSList *connection_attempts;
   GSList *successful_connections;
@@ -1485,7 +1487,12 @@ g_socket_client_async_connect_data_free (GSocketClientAsyncConnectData *data)
   data->task = NULL;
   g_clear_object (&data->connectable);
   g_clear_object (&data->enumerator);
+
+  g_cancellable_disconnect (data->enumeration_parent_cancellable, data->enumeration_cancelled_id);
+  g_clear_object (&data->enumeration_parent_cancellable);
+  data->enumeration_cancelled_id = 0;
   g_clear_object (&data->enumeration_cancellable);
+
   g_slist_free_full (data->connection_attempts, connection_attempt_unref);
   g_slist_free_full (data->successful_connections, connection_attempt_unref);
 
@@ -1503,6 +1510,7 @@ typedef struct
   GSocketClientAsyncConnectData *data; /* unowned */
   GSource *timeout_source;
   GCancellable *cancellable;
+  gulong cancelled_id;
   grefcount ref;
 } ConnectionAttempt;
 
@@ -1530,6 +1538,8 @@ connection_attempt_unref (gpointer pointer)
       g_clear_object (&attempt->address);
       g_clear_object (&attempt->socket);
       g_clear_object (&attempt->connection);
+      g_cancellable_disconnect (g_task_get_cancellable (attempt->data->task), attempt->cancelled_id);
+      attempt->cancelled_id = 0;
       g_clear_object (&attempt->cancellable);
       g_clear_object (&attempt->proxy_addr);
       if (attempt->timeout_source)
@@ -2023,8 +2033,9 @@ g_socket_client_enumerator_callback (GObject      *object,
   data->connection_attempts = g_slist_append (data->connection_attempts, attempt);
 
   if (g_task_get_cancellable (data->task))
-    g_cancellable_connect (g_task_get_cancellable (data->task), G_CALLBACK (on_connection_cancelled),
-                           g_object_ref (attempt->cancellable), g_object_unref);
+    attempt->cancelled_id =
+        g_cancellable_connect (g_task_get_cancellable (data->task), G_CALLBACK (on_connection_cancelled),
+                               g_object_ref (attempt->cancellable), g_object_unref);
 
   g_socket_connection_set_cached_remote_address ((GSocketConnection *)attempt->connection, address);
   g_debug ("GSocketClient: Starting TCP connection attempt");
@@ -2129,8 +2140,12 @@ g_socket_client_connect_async (GSocketClient       *client,
 
   data->enumeration_cancellable = g_cancellable_new ();
   if (cancellable)
-    g_cancellable_connect (cancellable, G_CALLBACK (on_connection_cancelled),
-                           g_object_ref (data->enumeration_cancellable), g_object_unref);
+    {
+      data->enumeration_parent_cancellable = g_object_ref (cancellable);
+      data->enumeration_cancelled_id =
+          g_cancellable_connect (cancellable, G_CALLBACK (on_connection_cancelled),
+                                 g_object_ref (data->enumeration_cancellable), g_object_unref);
+    }
 
   enumerator_next_async (data, FALSE);
 }
-- 
GitLab