luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-11-09 19:06:15 +01:00
Code Issues Packages Projects Releases Wiki Activity

gsocketclient: Fix still-reachable references to cancellables

Browse Source 
`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
This commit is contained in:
Philip Withnall 2022-06-13 13:06:06 +01:00
parent 6b65b721e7
commit 56d371942e
1 changed files with 19 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
gio/gsocketclient.c
View File

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