From 80af199d7de27fda251b4be9e1da3b53bd53e6fb Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Thu, 21 Feb 2019 15:31:52 -0500 Subject: [PATCH 1/2] gsocketclient: Fix critical on cancellation We need to be more explicit in handling cancellation to avoid multiple task returns. Fixes #1693 --- gio/gsocketclient.c | 35 ++++++++++++++----- gio/tests/gsocketclient-slow.c | 61 ++++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index 11e92e909..9db7ca0be 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -1338,6 +1338,8 @@ typedef struct GSList *connection_attempts; GError *last_error; + + gboolean completed; } GSocketClientAsyncConnectData; static void connection_attempt_unref (gpointer attempt); @@ -1424,9 +1426,15 @@ g_socket_client_async_connect_complete (GSocketClientAsyncConnectData *data) data->connection = (GIOStream *)wrapper_connection; } - g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, data->connection); - g_task_return_pointer (data->task, data->connection, g_object_unref); - data->connection = NULL; + if (!g_task_return_error_if_cancelled (data->task)) + { + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, data->connection); + g_task_return_pointer (data->task, g_steal_pointer (&data->connection), g_object_unref); + } + else + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL); + + data->completed = TRUE; g_object_unref (data->task); } @@ -1545,12 +1553,21 @@ g_socket_client_proxy_connect_callback (GObject *object, } static gboolean -task_completed_or_cancelled (GTask *task) +task_completed_or_cancelled (GSocketClientAsyncConnectData *data) { - if (g_task_get_completed (task)) + GTask *task = data->task; + GCancellable *cancellable = g_task_get_cancellable (task); + GError *error = NULL; + + if (data->completed) return TRUE; - else if (g_task_return_error_if_cancelled (task)) + else if (g_cancellable_set_error_if_cancelled (cancellable, &error)) + { + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL); + g_task_return_error (task, g_steal_pointer (&error)); + data->completed = TRUE; return TRUE; + } else return FALSE; } @@ -1567,7 +1584,7 @@ g_socket_client_connected_callback (GObject *source, GProxy *proxy; const gchar *protocol; - if (task_completed_or_cancelled (data->task) || g_cancellable_is_cancelled (attempt->cancellable)) + if (task_completed_or_cancelled (data) || g_cancellable_is_cancelled (attempt->cancellable)) { g_object_unref (data->task); connection_attempt_unref (attempt); @@ -1701,7 +1718,7 @@ g_socket_client_enumerator_callback (GObject *object, ConnectionAttempt *attempt; GError *error = NULL; - if (task_completed_or_cancelled (data->task)) + if (task_completed_or_cancelled (data)) { g_object_unref (data->task); return; @@ -1718,6 +1735,7 @@ g_socket_client_enumerator_callback (GObject *object, } g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL); + data->completed = TRUE; if (!error) { if (data->last_error) @@ -1835,6 +1853,7 @@ g_socket_client_connect_async (GSocketClient *client, */ data->task = g_task_new (client, cancellable, callback, user_data); + g_task_set_check_cancellable (data->task, FALSE); /* We handle this manually */ g_task_set_source_tag (data->task, g_socket_client_connect_async); g_task_set_task_data (data->task, data, (GDestroyNotify)g_socket_client_async_connect_data_free); diff --git a/gio/tests/gsocketclient-slow.c b/gio/tests/gsocketclient-slow.c index a78ea71d0..4143bc3d8 100644 --- a/gio/tests/gsocketclient-slow.c +++ b/gio/tests/gsocketclient-slow.c @@ -87,7 +87,21 @@ on_timer (GCancellable *cancel) } static void -test_happy_eyeballs_cancel (void) +on_event (GSocketClient *client, + GSocketClientEvent event, + GSocketConnectable *connectable, + GIOStream *connection, + gboolean *got_completed_event) +{ + if (event == G_SOCKET_CLIENT_COMPLETE) + { + *got_completed_event = TRUE; + g_assert_null (connection); + } +} + +static void +test_happy_eyeballs_cancel_delayed (void) { GSocketClient *client; GSocketService *service; @@ -95,6 +109,10 @@ test_happy_eyeballs_cancel (void) guint16 port; GMainLoop *loop; GCancellable *cancel; + gboolean got_completed_event = FALSE; + + /* This just tests that cancellation works as expected, still emits the completed signal, + * and never returns a connection */ loop = g_main_loop_new (NULL, FALSE); @@ -107,8 +125,45 @@ test_happy_eyeballs_cancel (void) cancel = g_cancellable_new (); g_socket_client_connect_to_host_async (client, "localhost", port, cancel, on_connected_cancelled, loop); g_timeout_add (1, (GSourceFunc) on_timer, cancel); + g_signal_connect (client, "event", G_CALLBACK (on_event), &got_completed_event); g_main_loop_run (loop); + g_assert_true (got_completed_event); + g_main_loop_unref (loop); + g_object_unref (service); + g_object_unref (client); + g_object_unref (cancel); +} + +static void +test_happy_eyeballs_cancel_instant (void) +{ + GSocketClient *client; + GSocketService *service; + GError *error = NULL; + guint16 port; + GMainLoop *loop; + GCancellable *cancel; + gboolean got_completed_event = FALSE; + + /* This tests the same things as above, test_happy_eyeballs_cancel_delayed(), but + * with different timing since it sends an already cancelled cancellable */ + + loop = g_main_loop_new (NULL, FALSE); + + service = g_socket_service_new (); + port = g_socket_listener_add_any_inet_port (G_SOCKET_LISTENER (service), NULL, &error); + g_assert_no_error (error); + g_socket_service_start (service); + + client = g_socket_client_new (); + cancel = g_cancellable_new (); + g_cancellable_cancel (cancel); + g_socket_client_connect_to_host_async (client, "localhost", port, cancel, on_connected_cancelled, loop); + g_signal_connect (client, "event", G_CALLBACK (on_event), &got_completed_event); + g_main_loop_run (loop); + + g_assert_true (got_completed_event); g_main_loop_unref (loop); g_object_unref (service); g_object_unref (client); @@ -121,7 +176,9 @@ main (int argc, char *argv[]) g_test_init (&argc, &argv, NULL); g_test_add_func ("/socket-client/happy-eyeballs/slow", test_happy_eyeballs); - g_test_add_func ("/socket-client/happy-eyeballs/cancellation", test_happy_eyeballs_cancel); + 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); + return g_test_run (); } \ No newline at end of file From f0fcb68da5d61b0b8d06916eb0edf76acea56b25 Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Fri, 22 Feb 2019 12:25:13 -0500 Subject: [PATCH 2/2] tests: Unmark gsocketclient-slow as flaky Closes #1653 --- gio/tests/meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/gio/tests/meson.build b/gio/tests/meson.build index b5b41bb7c..59d11ee96 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -150,7 +150,6 @@ if host_machine.system() != 'windows' 'installed_tests_env' : { 'LD_PRELOAD': '@0@/slow-connect-preload.so'.format(installed_tests_execdir), }, - 'suite': ['flaky'], }, 'gschema-compile' : {'install' : false}, 'trash' : {},