[Spice-commits] 2 commits - src/spice-channel.c src/vmcstream.c

[Victor Toso de Carvalho]

 src/spice-channel.c |    2 ++
 src/vmcstream.c     |   30 +++++++++++++++---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

New commits:
commit 42a5ac811769dc1e19531b769f8df652a67d84df
Author: Victor Toso <victortoso at redhat.com>
Date:   Mon May 2 16:00:54 2016 +0200

    spice-channel: fix small leak
    
    182 bytes in 1 blocks are definitely lost in loss record 9,048 of 9,889
    at 0x4C2A988: calloc (vg_replace_malloc.c:711)
    by 0xB5F4270: g_malloc0 (gmem.c:124)
    by 0x7320678: spice_channel_recv_link_hdr (spice-channel.c:1312)
    by 0x7320678: spice_channel_coroutine (spice-channel.c:2543)
    by 0x7355A4E: coroutine_trampoline (coroutine_ucontext.c:63)
    by 0x7355802: continuation_trampoline (continuation.c:55)
    
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 7b0a3dc..e81034f 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -187,6 +187,8 @@ static void spice_channel_finalize(GObject *gobject)
     if (c->remote_common_caps)
         g_array_free(c->remote_common_caps, TRUE);
 
+    g_clear_pointer(&c->peer_msg, g_free);
+
     /* Chain up to the parent class */
     if (G_OBJECT_CLASS(spice_channel_parent_class)->finalize)
         G_OBJECT_CLASS(spice_channel_parent_class)->finalize(gobject);
commit 9e58e5024741da99dfda78496717e3863e8ef6bc
Author: Victor Toso <victortoso at redhat.com>
Date:   Mon May 2 15:06:09 2016 +0200

    spicevmc: don't disconnect on "cancel"
    
    spicevmc was designed so its _finish functions should always be called;
    With the gtask integration both _finish functions are trying to
    disconnect the GCancellable even in the 'cancel' context which leads to
    a deadlock as explained in the documentation.
    
    Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=95032
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/src/vmcstream.c b/src/vmcstream.c
index 5dd2799..4963de4 100644
--- a/src/vmcstream.c
+++ b/src/vmcstream.c
@@ -183,13 +183,10 @@ read_cancelled(GCancellable *cancellable,
                             G_IO_ERROR, G_IO_ERROR_CANCELLED,
                             "read cancelled");
 
+    /* With GTask, we don't need to disconnect GCancellable when task is
+     * cancelled within cancellable callback as it could lead to deadlocks
+     * e.g: https://bugzilla.gnome.org/show_bug.cgi?id=705395 */
     g_clear_object(&self->task);
-
-    /* See FIXME */
-    /* if (self->cancellable) { */
-    /*     g_cancellable_disconnect(self->cancellable, self->cancel_id); */
-    /*     g_clear_object(&self->cancellable); */
-    /* } */
 }
 
 G_GNUC_INTERNAL void
@@ -230,13 +227,14 @@ spice_vmc_input_stream_read_all_finish(GInputStream *stream,
 {
     GTask *task = G_TASK(result);
     SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
+    GCancellable *cancel;
 
     g_return_val_if_fail(g_task_is_valid(task, self), -1);
-
-    /* FIXME: calling _finish() is required. Disconnecting in
-       read_cancelled() causes a deadlock. #705395 */
-    g_cancellable_disconnect(g_task_get_cancellable(task), self->cancel_id);
-
+    cancel = g_task_get_cancellable(task);
+    if (!g_cancellable_is_cancelled(cancel)) {
+         g_cancellable_disconnect(cancel, self->cancel_id);
+         self->cancel_id = 0;
+    }
     return g_task_propagate_int(task, error);
 }
 
@@ -276,13 +274,15 @@ spice_vmc_input_stream_read_finish(GInputStream *stream,
 {
     GTask *task = G_TASK(result);
     SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
+    GCancellable *cancel;
 
     g_return_val_if_fail(g_task_is_valid(task, self), -1);
 
-    /* FIXME: calling _finish() is required. Disconnecting in
-       read_cancelled() causes a deadlock. #705395 */
-    g_cancellable_disconnect(g_task_get_cancellable(task), self->cancel_id);
-
+    cancel = g_task_get_cancellable(task);
+    if (!g_cancellable_is_cancelled(cancel)) {
+         g_cancellable_disconnect(cancel, self->cancel_id);
+         self->cancel_id = 0;
+    }
     return g_task_propagate_int(task, error);
 }