From 1942dd5b5c91fdab96e7c102a3f73a4190035e19 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Nov 2023 11:14:56 +0000 Subject: [PATCH 1/2] gsocketclient: Fix a leak of the task data on an error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the task is completed (and `g_task_return_*()` has been called), the task is no longer needed. It would make more sense to unref it in `complete_connection_with_error()`, where `g_task_return_*()` is called, but that complicates other call sites significantly, so I didn’t do it. Signed-off-by: Philip Withnall Fixes: #3184 --- gio/gsocketclient.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index d4231599a..f684986fb 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -1919,6 +1919,7 @@ try_next_connection_or_finish (GSocketClientAsyncConnectData *data, } complete_connection_with_error (data, g_steal_pointer (&data->error_info->best_error)); + g_object_unref (data->task); } static void From 9fccda086a01195cfb481d1d787b0db0c20ddd06 Mon Sep 17 00:00:00 2001 From: Johan Sternerup Date: Mon, 3 Jun 2024 15:23:14 +0200 Subject: [PATCH 2/2] gsocketclient: Add unit test for leak of task data in error path The unit test cover the error path that causes the leak described in https://gitlab.gnome.org/GNOME/glib/-/issues/3184. --- gio/tests/gsocketclient-slow.c | 67 +++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/gio/tests/gsocketclient-slow.c b/gio/tests/gsocketclient-slow.c index f56e11d25..18781eff9 100644 --- a/gio/tests/gsocketclient-slow.c +++ b/gio/tests/gsocketclient-slow.c @@ -19,6 +19,7 @@ */ #include +#include /* For IPV6_V6ONLY */ static void on_connected (GObject *source_object, @@ -173,6 +174,70 @@ test_happy_eyeballs_cancel_instant (void) g_object_unref (cancel); } +static void +async_result_cb (GObject *source, + GAsyncResult *res, + gpointer user_data) +{ + GAsyncResult **result_out = user_data; + + g_assert_null (*result_out); + *result_out = g_object_ref (res); + + g_main_context_wakeup (NULL); +} + +static void +test_connection_failed (void) +{ + GSocketClient *client = NULL; + GInetAddress *inet_address = NULL; + GSocketAddress *address = NULL; + GSocket *socket = NULL; + guint16 port; + GAsyncResult *async_result = NULL; + GSocketConnection *conn = NULL; + GError *local_error = NULL; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3184"); + + inet_address = g_inet_address_new_any (G_SOCKET_FAMILY_IPV6); + address = g_inet_socket_address_new (inet_address, 0); + g_object_unref (inet_address); + + socket = g_socket_new (G_SOCKET_FAMILY_IPV6, G_SOCKET_TYPE_STREAM, G_SOCKET_PROTOCOL_TCP, &local_error); + g_assert_no_error (local_error); + g_socket_set_option (socket, IPPROTO_IPV6, IPV6_V6ONLY, FALSE, NULL); + g_socket_bind (socket, address, TRUE, &local_error); + g_assert_no_error (local_error); + + /* reserve a port without listening so we know that connecting to it will fail */ + port = g_inet_socket_address_get_port (G_INET_SOCKET_ADDRESS (address)); + + client = g_socket_client_new (); + /* Connect to the port we have reserved but do not listen to. Because of the slow connection + * caused by slow-connect-preload.c and the fact that we try to connect to both IPv4 and IPv6 + * we will in some way exercise the code path in try_next_connection_or_finish() that ends + * with a call to complete_connection_with_error(). This path previously had a memory leak. + * Note that the slowness is important, because without it we could bail out already in the + * address enumeration phase because it finishes when there are no connection attempts in + * progress. */ + g_socket_client_connect_to_host_async (client, "localhost", port, NULL, async_result_cb, &async_result); + + while (async_result == NULL) + g_main_context_iteration (NULL, TRUE); + + conn = g_socket_client_connect_to_uri_finish (client, async_result, &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_CONNECTION_REFUSED); + g_assert_null (conn); + g_clear_error (&local_error); + g_clear_object (&async_result); + + g_clear_object (&client); + g_clear_object (&socket); + g_clear_object (&address); +} + int main (int argc, char *argv[]) { @@ -181,7 +246,7 @@ main (int argc, char *argv[]) g_test_add_func ("/socket-client/happy-eyeballs/slow", test_happy_eyeballs); g_test_add_func ("/socket-client/happy-eyeballs/cancellation/instant", test_happy_eyeballs_cancel_instant); g_test_add_func ("/socket-client/happy-eyeballs/cancellation/delayed", test_happy_eyeballs_cancel_delayed); - + g_test_add_func ("/socket-client/connection-fail", test_connection_failed); return g_test_run (); }