[glib-networking/mcatanzaro/test-read-timeout-write: 3/3] tests: fix and reenable test_connection_read_time_out_write()

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 6f223882bbd6b783d6fced9e2d9c2c3c31b14e4f
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Thu May 16 20:34:07 2019 -0500

    tests: fix and reenable test_connection_read_time_out_write()
    
    We have a fun non-mutex deadlock here (backtrace below). Problem is
    that after the client's handshake thread completes, then
    handshake_thread_completed() is called in the client, during which
    g_task_return_boolean() will be called to end the asynchronous operation
    started with g_tls_connection_base_handshake_async(). Everything goes
    fine if g_task_return() decides to execute its callback on the next
    iteration of the main context, which is what usually happens when
    running this test. But sometimes it executes its callback immediately
    instead, which is legal, documentated behavior of GTask. So we need to
    be prepared for this to happen. When it does, the test's
    socket_client_timed_out_write() is then executed, and control will not
    return to the main context until this function call completes. i.e. the
    main context iteration cannot proceed until
    socket_client_timed_out_write() is finished. But this callback is
    performing a synchronous read, first to test that the read works
    properly, then to test what happens when it times out. The problem is
    the *first* read will time out, because it can't proceed until the
    server writes data. But the server cannot write data, because its main
    context is blocked waiting for the client to read the data that is not
    forthcoming. So the first read will time out instead. Drat!
    
    This is the only affected test because this is the only test that tests
    writing after an explicit async handshake. (GSocketClient is kind enough
    to perform an explicit handshake for us. Most of our tests trigger
    implicit handshakes instead.)
    
     (gdb) bt
     #0  0x00007ffff7a6eeb5 in raise () from /lib64/libc.so.6
     #1  0x00007ffff7a59895 in abort () from /lib64/libc.so.6
     #2  0x00007ffff7ca8e0f in g_assertion_message (domain=0x40d07c "GLib-Net",
         file=0x40d040 "../../../../Projects/glib-networking/tls/tests/connection.c", line=1480,
         func=0x40e2d0 <__func__.28691> "socket_client_timed_out_write",
         message=0x5c4d10 "assertion failed (error == NULL): Socket I/O timed out (g-io-error-quark, 24)")
         at ../../../../Projects/glib/glib/gtestutils.c:2860
     #3  0x00007ffff7ca92ea in g_assertion_message_error (domain=0x40d07c "GLib-Net",
         file=0x40d040 "../../../../Projects/glib-networking/tls/tests/connection.c", line=1480,
         func=0x40e2d0 <__func__.28691> "socket_client_timed_out_write", expr=0x40d0e8 "error", 
error=0x460b00,
         error_domain=0, error_code=0) at ../../../../Projects/glib/glib/gtestutils.c:2975
     #4  0x0000000000408de5 in socket_client_timed_out_write (source=0x413960, result=0x454100, 
user_data=0x449580)
         at ../../../../Projects/glib-networking/tls/tests/connection.c:1480
     #5  0x00007ffff7e848ff in g_task_return_now (task=0x454100) at ../../../../Projects/glib/gio/gtask.c:1209
     #6  0x00007ffff7e84ab8 in g_task_return (task=0x454100, type=G_TASK_RETURN_SUCCESS)
         at ../../../../Projects/glib/gio/gtask.c:1281
     #7  0x00007ffff7e85585 in g_task_return_pointer (task=0x454100, result=0x8e1890,
         result_destroy=0x7ffff7d7d45b <g_object_unref>) at ../../../../Projects/glib/gio/gtask.c:1686
     #8  0x00007ffff7e774ff in g_socket_client_async_connect_complete (data=0x452ca0)
         at ../../../../Projects/glib/gio/gsocketclient.c:1442
     #9  0x00007ffff7e77718 in g_socket_client_tls_handshake_callback (object=0x4731b0, result=0x454280,
         user_data=0x452ca0) at ../../../../Projects/glib/gio/gsocketclient.c:1501
     #10 0x00007ffff7e848ff in g_task_return_now (task=0x454280) at ../../../../Projects/glib/gio/gtask.c:1209
     #11 0x00007ffff7e84ab8 in g_task_return (task=0x454280, type=G_TASK_RETURN_SUCCESS)
         at ../../../../Projects/glib/gio/gtask.c:1281
     #12 0x00007ffff7e85961 in g_task_return_boolean (task=0x454280, result=1)
         at ../../../../Projects/glib/gio/gtask.c:1798
     #13 0x00007ffff7c14bb4 in handshake_thread_completed (object=0x4731b0, result=0x454040, 
user_data=0x454280)
         at ../../../../Projects/glib-networking/tls/base/gtlsconnection-base.c:1613
     #14 0x00007ffff7e848ff in g_task_return_now (task=0x454040) at ../../../../Projects/glib/gio/gtask.c:1209
     #15 0x00007ffff7e84996 in complete_in_idle_cb (task=0x454040) at 
../../../../Projects/glib/gio/gtask.c:1225
     #16 0x00007ffff7c7bb96 in g_idle_dispatch (source=0x7fffec006b20, callback=0x7ffff7e8494d 
<complete_in_idle_cb>,
         user_data=0x454040) at ../../../../Projects/glib/glib/gmain.c:5633
     #17 0x00007ffff7c78f95 in g_main_dispatch (context=0x4496f0) at 
../../../../Projects/glib/glib/gmain.c:3196
     #18 0x00007ffff7c79e8d in g_main_context_dispatch (context=0x4496f0) at 
../../../../Projects/glib/glib/gmain.c:3862
     #19 0x00007ffff7c7a071 in g_main_context_iterate (context=0x4496f0, block=1, dispatch=1, self=0x448960)
         at ../../../../Projects/glib/glib/gmain.c:3935
     #20 0x00007ffff7c7a4a0 in g_main_loop_run (loop=0x428070) at ../../../../Projects/glib/glib/gmain.c:4129
     #21 0x00000000004090ca in test_connection_read_time_out_write (test=0x449580, data=0x0)
         at ../../../../Projects/glib-networking/tls/tests/connection.c:1523
     #22 0x00007ffff7ca82f4 in test_case_run (tc=0x42baa0) at ../../../../Projects/glib/glib/gtestutils.c:2579
     #23 0x00007ffff7ca86b0 in g_test_run_suite_internal (suite=0x4208c0,
         path=0x7fffffffd838 "/tls/connection/read-time-out-then-write")
         at ../../../../Projects/glib/glib/gtestutils.c:2667
     #24 0x00007ffff7ca8759 in g_test_run_suite_internal (suite=0x4208a0,
         path=0x7fffffffd838 "/tls/connection/read-time-out-then-write")
         at ../../../../Projects/glib/glib/gtestutils.c:2679
     #25 0x00007ffff7ca8759 in g_test_run_suite_internal (suite=0x420880,
         path=0x7fffffffd838 "/tls/connection/read-time-out-then-write")
         at ../../../../Projects/glib/glib/gtestutils.c:2679
     #26 0x00007ffff7ca8947 in g_test_run_suite (suite=0x420880) at 
../../../../Projects/glib/glib/gtestutils.c:2751
     #27 0x00007ffff7ca754c in g_test_run () at ../../../../Projects/glib/glib/gtestutils.c:1989
     #28 0x000000000040bcc1 in main (argc=1, argv=0x7fffffffd448)
         at ../../../../Projects/glib-networking/tls/tests/connection.c:2287

 tls/tests/connection.c | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)
---
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index 3588484..d62d16d 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -1453,25 +1453,16 @@ test_connection_socket_client_failed (TestConnection *test,
   g_object_unref (client);
 }
 
-#if 0
-static void
-socket_client_timed_out_write (GObject      *source,
-                               GAsyncResult *result,
-                               gpointer      user_data)
+static gboolean
+socket_client_timed_out_write (gpointer user_data)
 {
   TestConnection *test = user_data;
-  GSocketConnection *connection;
   GInputStream *input_stream;
   GOutputStream *output_stream;
   GError *error = NULL;
   gchar buffer[TEST_DATA_LENGTH];
   gssize size;
 
-  connection = g_socket_client_connect_finish (G_SOCKET_CLIENT (source),
-                                               result, &error);
-  g_assert_no_error (error);
-  test->client_connection = G_IO_STREAM (connection);
-
   input_stream = g_io_stream_get_input_stream (test->client_connection);
   output_stream = g_io_stream_get_output_stream (test->client_connection);
 
@@ -1495,16 +1486,40 @@ socket_client_timed_out_write (GObject      *source,
   g_assert_cmpint (size, ==, TEST_DATA_LENGTH);
 
   g_main_loop_quit (test->loop);
+
+  return G_SOURCE_REMOVE;
+}
+
+static void
+socket_client_timed_out_write_connected (GObject      *source,
+                                         GAsyncResult *result,
+                                         gpointer      user_data)
+{
+  TestConnection *test = user_data;
+  GSocketConnection *connection;
+  GError *error = NULL;
+
+  connection = g_socket_client_connect_finish (G_SOCKET_CLIENT (source),
+                                               result, &error);
+  g_assert_no_error (error);
+  test->client_connection = G_IO_STREAM (connection);
+
+  /* We need to use an idle callback here to guarantee that the upcoming call
+   * to g_input_stream_read() executes on the next iteration of the main
+   * context. Otherwise, we could deadlock ourselves: the read would not be able
+   * to complete if GTask executes socket_client_timed_out_write_connected()
+   * using g_task_return_now() instead of posting the invocation to the next
+   * iteration of the main context, because the server will not progress until
+   * the main context is iterated, but iteration would be blocked waiting for
+   * client's read to complete.
+   */
+  g_idle_add (socket_client_timed_out_write, test);
 }
-#endif
 
 static void
 test_connection_read_time_out_write (TestConnection *test,
                                      gconstpointer   data)
 {
-#if 0
-  // FIXME: This test is broken.
-
   GSocketClient *client;
   GTlsCertificateFlags flags;
   GSocketConnection *connection;
@@ -1523,7 +1538,7 @@ test_connection_read_time_out_write (TestConnection *test,
   g_socket_client_set_tls_validation_flags (client, flags);
 
   g_socket_client_connect_async (client, G_SOCKET_CONNECTABLE (test->address),
-                                 NULL, socket_client_timed_out_write, test);
+                                 NULL, socket_client_timed_out_write_connected, test);
 
   g_main_loop_run (test->loop);
 
@@ -1542,7 +1557,6 @@ test_connection_read_time_out_write (TestConnection *test,
   g_object_unref (connection);
 
   g_object_unref (client);
-#endif
 }
 
 static void