[Spice-commits] 3 commits - src/wocky-http-proxy.c

Christophe Fergau teuf at kemper.freedesktop.org
Fri Aug 5 06:29:15 UTC 2016


 src/wocky-http-proxy.c |   60 +++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

New commits:
commit 60c5718f64ff33e565d4cfcd6550f50e77547580
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Aug 3 09:17:27 2016 +0200

    wocky: Pass GTask around instead of ConnectAsyncData
    
    Now that the ConnectAsyncData we need is set as GTask data, we can pass
    around the GTask rather than the ConnectAsyncData instance, and remove
    the GTask pointer from it. We can get the ConnectAsyncData from the
    GTask when needed using g_task_get_task_data().

diff --git a/src/wocky-http-proxy.c b/src/wocky-http-proxy.c
index cf51ba3..8120a55 100644
--- a/src/wocky-http-proxy.c
+++ b/src/wocky-http-proxy.c
@@ -252,7 +252,6 @@ error:
 
 typedef struct
 {
-  GTask *task;
   GIOStream *io_stream;
   gchar *buffer;
   gssize length;
@@ -283,35 +282,35 @@ free_connect_data (ConnectAsyncData *data)
 }
 
 static void
-complete_async_from_error (ConnectAsyncData *data, GError *error)
+complete_async_from_error (GTask *task, GError *error)
 {
-  GTask *task = data->task;
-
   if (error == NULL)
     g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_PROXY_FAILED,
         "HTTP proxy server closed connection unexpectedly.");
 
-  g_task_return_error(data->task, error);
+  g_task_return_error (task, error);
   g_object_unref (task);
 }
 
 static void
-do_write (GAsyncReadyCallback callback, ConnectAsyncData *data)
+do_write (GAsyncReadyCallback callback, GTask *task)
 {
   GOutputStream *out;
+  ConnectAsyncData *data = g_task_get_task_data (task);
   out = g_io_stream_get_output_stream (data->io_stream);
   g_output_stream_write_async (out,
       data->buffer + data->offset,
       data->length - data->offset,
-      G_PRIORITY_DEFAULT, g_task_get_cancellable(data->task),
-      callback, data);
+      G_PRIORITY_DEFAULT, g_task_get_cancellable(task),
+      callback, task);
 }
 
 static void
-stream_connected (ConnectAsyncData *data,
+stream_connected (GTask *task,
                   GIOStream *io_stream)
 {
   GInputStream *in;
+  ConnectAsyncData *data = g_task_get_task_data (task);
 
   data->io_stream = g_object_ref (io_stream);
   in = g_io_stream_get_input_stream (io_stream);
@@ -319,7 +318,7 @@ stream_connected (ConnectAsyncData *data,
   g_filter_input_stream_set_close_base_stream (G_FILTER_INPUT_STREAM (data->data_in),
                                                FALSE);
 
-  do_write (request_write_cb, data);
+  do_write (request_write_cb, task);
 }
 
 static void
@@ -328,16 +327,16 @@ handshake_completed (GObject *source_object,
                      gpointer user_data)
 {
   GTlsConnection *conn = G_TLS_CONNECTION (source_object);
-  ConnectAsyncData *data = user_data;
+  GTask *task = G_TASK (user_data);
   GError *error = NULL;
 
   if (!g_tls_connection_handshake_finish (conn, res, &error))
     {
-      complete_async_from_error (data, error);
+      complete_async_from_error (task, error);
       return;
     }
 
-  stream_connected (data, G_IO_STREAM (conn));
+  stream_connected (task, G_IO_STREAM (conn));
 }
 
 static void
@@ -357,7 +356,6 @@ wocky_http_proxy_connect_async (GProxy *proxy,
                      user_data);
 
   data = g_new0 (ConnectAsyncData, 1);
-  data->task = task;
 
   data->buffer = create_request (proxy_address, &data->has_cred);
   data->length = strlen (data->buffer);
@@ -375,7 +373,7 @@ wocky_http_proxy_connect_async (GProxy *proxy,
                                              &error);
       if (!tlsconn)
         {
-          complete_async_from_error (data, error);
+          complete_async_from_error (task, error);
           return;
         }
 
@@ -389,11 +387,11 @@ wocky_http_proxy_connect_async (GProxy *proxy,
                                                     tls_validation_flags);
       g_tls_connection_handshake_async (G_TLS_CONNECTION (tlsconn),
                                         G_PRIORITY_DEFAULT, cancellable,
-                                        handshake_completed, data);
+                                        handshake_completed, task);
     }
   else
     {
-      stream_connected (data, io_stream);
+      stream_connected (task, io_stream);
     }
 }
 
@@ -403,14 +401,15 @@ request_write_cb (GObject *source,
     gpointer user_data)
 {
   GError *error = NULL;
-  ConnectAsyncData *data = user_data;
+  GTask *task = G_TASK(user_data);
+  ConnectAsyncData *data = g_task_get_task_data (task);
   gssize written;
 
   written = g_output_stream_write_finish (G_OUTPUT_STREAM (source),
       res, &error);
   if (written < 0)
     {
-      complete_async_from_error (data, error);
+      complete_async_from_error (task, error);
       return;
     }
 
@@ -423,13 +422,13 @@ request_write_cb (GObject *source,
       g_data_input_stream_read_until_async (data->data_in,
           HTTP_END_MARKER,
           G_PRIORITY_DEFAULT,
-          g_task_get_cancellable(data->task),
-          reply_read_cb, data);
+          g_task_get_cancellable(task),
+          reply_read_cb, task);
 
     }
   else
     {
-      do_write (request_write_cb, data);
+      do_write (request_write_cb, task);
     }
 }
 
@@ -439,26 +438,27 @@ reply_read_cb (GObject *source,
     gpointer user_data)
 {
   GError *error = NULL;
-  ConnectAsyncData *data = user_data;
+  GTask *task = G_TASK(user_data);
+  ConnectAsyncData *data = g_task_get_task_data (task);
 
   data->buffer = g_data_input_stream_read_until_finish (data->data_in,
       res, NULL, &error);
 
   if (data->buffer == NULL)
     {
-      complete_async_from_error (data, error);
+      complete_async_from_error (task, error);
       return;
     }
 
   if (!check_reply (data->buffer, data->has_cred, &error))
     {
-      complete_async_from_error (data, error);
+      complete_async_from_error (task, error);
       return;
     }
 
-  g_task_return_pointer (data->task, data->io_stream, (GDestroyNotify) g_object_unref);
+  g_task_return_pointer (task, data->io_stream, (GDestroyNotify) g_object_unref);
   data->io_stream = NULL;
-  g_object_unref (data->task);
+  g_object_unref (task);
 }
 
 static GIOStream *
commit 8bba28466b6c240e25676adc818e08bc6e4ef562
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Aug 3 09:06:49 2016 +0200

    wocky: Set ConnectAsyncData instance as GTask data
    
    Currently, the ConnectAsyncData instance is leaked if for example
    we trigger one codepath calling g_task_return_error(). If we
    associate it with the GTask with g_task_set_task_data(),
    this kind of leak will be avoided.

diff --git a/src/wocky-http-proxy.c b/src/wocky-http-proxy.c
index d09cf41..cf51ba3 100644
--- a/src/wocky-http-proxy.c
+++ b/src/wocky-http-proxy.c
@@ -363,6 +363,8 @@ wocky_http_proxy_connect_async (GProxy *proxy,
   data->length = strlen (data->buffer);
   data->offset = 0;
 
+  g_task_set_task_data (task, data, (GDestroyNotify)free_connect_data);
+
   if (WOCKY_IS_HTTPS_PROXY (proxy))
     {
       GError *error = NULL;
@@ -457,7 +459,6 @@ reply_read_cb (GObject *source,
   g_task_return_pointer (data->task, data->io_stream, (GDestroyNotify) g_object_unref);
   data->io_stream = NULL;
   g_object_unref (data->task);
-  free_connect_data (data);
 }
 
 static GIOStream *
commit 57ba17589bb8aa2a20becc9aeec7675669cdd6d5
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Aug 3 08:59:19 2016 +0200

    wocky: Simplify wocky_http_proxy_connect_finish()
    
    Rather than returning the whole ConnectAsyncData struct with
    g_task_return_pointer(), we can return only the GIOStream object as this
    is what we are interested in.
    
    This has the side-effect of fixing a ConnectAsyncData leak as after
    calling g_task_propagate_pointer() the old code had ownership of the
    ConnectAsyncData instance but was never freeing it.
    
    The leak is:
    ==20010== 4,348 (56 direct, 4,292 indirect) bytes in 1 blocks are definitely lost in loss record 20,762 of 20,999
    ==20010==    at 0x4C2DA60: calloc (vg_replace_malloc.c:711)
    ==20010==    by 0xD0F6EB0: g_malloc0 (gmem.c:124)
    ==20010==    by 0x75C0978: wocky_http_proxy_connect_async (wocky-http-proxy.c:359)
    ==20010==    by 0xCB4E22C: g_socket_client_connected_callback (gsocketclient.c:1548)
    ==20010==    by 0xCB57342: g_task_return_now (gtask.c:1107)
    ==20010==    by 0xCB579E5: g_task_return (gtask.c:1165)
    ==20010==    by 0xCB4FB1C: g_socket_connection_connect_callback (gsocketconnection.c:236)
    ==20010==    by 0xCB47160: socket_source_dispatch (gsocket.c:3543)
    ==20010==    by 0xD0F1702: g_main_dispatch (gmain.c:3154)
    ==20010==    by 0xD0F1702: g_main_context_dispatch (gmain.c:3769)
    ==20010==    by 0xD0F1AAF: g_main_context_iterate.isra.29 (gmain.c:3840)
    ==20010==    by 0xD0F1B5B: g_main_context_iteration (gmain.c:3901)
    ==20010==    by 0xCB7D58C: g_application_run (gapplication.c:2381)
    ==20010==    by 0x41571C: main (remote-viewer-main.c:42)

diff --git a/src/wocky-http-proxy.c b/src/wocky-http-proxy.c
index f62f1fb..d09cf41 100644
--- a/src/wocky-http-proxy.c
+++ b/src/wocky-http-proxy.c
@@ -454,8 +454,10 @@ reply_read_cb (GObject *source,
       return;
     }
 
-  g_task_return_pointer (data->task, data, (GDestroyNotify) free_connect_data);
+  g_task_return_pointer (data->task, data->io_stream, (GDestroyNotify) g_object_unref);
+  data->io_stream = NULL;
   g_object_unref (data->task);
+  free_connect_data (data);
 }
 
 static GIOStream *
@@ -464,9 +466,8 @@ wocky_http_proxy_connect_finish (GProxy *proxy,
     GError **error)
 {
   GTask *task = G_TASK (result);
-  ConnectAsyncData *data = g_task_propagate_pointer (task, error);
 
-  return data ? g_object_ref (data->io_stream) : NULL;
+  return g_task_propagate_pointer (task, error);
 }
 
 static gboolean


More information about the Spice-commits mailing list