[glib: 1/2] gsocketclient: Fix still-reachable references to cancellables




commit 56d371942e43c52bc6131067e2dc2a35f6cd5a3d
Author: Philip Withnall <pwithnall endlessos org>
Date:   Mon Jun 13 13:06:06 2022 +0100

    gsocketclient: Fix still-reachable references to cancellables
    
    `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

 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);
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]