[Spice-devel] [spice-gtk v2 3/9] file-transfer: Fix GTask leaks

[Victor Toso]

Hi,

On Tue, Aug 02, 2016 at 11:48:44AM +0200, Victor Toso wrote:
> From: Christophe Fergeau <cfergeau at redhat.com>
>
> The file transfer code calls g_*_async() methods with a GTask as the
> user_data argument. It needs to own a reference to the GTask for the
> duration of the async call to keep the GTask alive, however it was never
> releasing it when it no longer needs it (that is, after calling
> g_task_return_*).
> 
> Some of the leaks fixed by this patch are:
> 
> * GTask leak after flush
> 11,232 bytes in 54 blocks are definitely lost in
> loss record 14,018 of 14,071
>  at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
>  by 0xC14A0E8: g_malloc (gmem.c:94)
>  by 0xC1608A2: g_slice_alloc (gslice.c:1025)
>  by 0xC160ECD: g_slice_alloc0 (gslice.c:1051)
>  by 0xBEDBD11: g_type_create_instance (gtype.c:1857)
>  by 0xBEBD7DA: g_object_new_internal (gobject.c:1781)
>  by 0xBEBF11C: g_object_newv (gobject.c:1928)
>  by 0xBEBF89B: g_object_new (gobject.c:1621)
>  by 0xBBA4424: g_task_new (gtask.c:693)
>  by 0x6F8C3C0: file_xfer_flush_async (channel-main.c:899)
>  by 0x6F8C3C0: file_xfer_read_async_cb (channel-main.c:1826)
>  by 0xBBA3F38: g_task_return_now (gtask.c:1121)
>  by 0xBBA4775: g_task_return (gtask.c:1179)
> 
> * GTask leak from spice_main_file_copy_async()
> 208 bytes in 1 blocks are definitely lost in
> loss record 13,708 of 15,093
>  at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
>  by 0xC14A0E8: g_malloc (gmem.c:94)
>  by 0xC1608A2: g_slice_alloc (gslice.c:1025)
>  by 0xC160ECD: g_slice_alloc0 (gslice.c:1051)
>  by 0xBEDBD11: g_type_create_instance (gtype.c:1857)
>  by 0xBEBD7DA: g_object_new_internal (gobject.c:1781)
>  by 0xBEBF11C: g_object_newv (gobject.c:1928)
>  by 0xBEBF89B: g_object_new (gobject.c:1621)
>  by 0xBBA4424: g_task_new (gtask.c:693)
>  by 0x6F8EF55: spice_main_file_copy_async (channel-main.c:3084)
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>

I've included in the commit log this two leaks and its fixes to the
patch.

Acked-by: Victor Toso <victortoso at redhat.com>

> ---
>  src/channel-main.c             | 2 ++
>  src/spice-file-transfer-task.c | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 26192ed..5af73b4 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -936,6 +936,7 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
>          if (task) {
>              /* if there's a flush task waiting for this message, finish it */
>              g_task_return_boolean(task, TRUE);
> +            g_object_unref(task);
>              g_hash_table_remove(c->flushing, out);
>          }
>      }
> @@ -2887,6 +2888,7 @@ static void file_transfer_operation_free(FileTransferOperation *xfer_op)
>          SPICE_DEBUG("Transfer successful (%p)", xfer_op);
>          g_task_return_boolean(xfer_op->task, TRUE);
>      }
> +    g_object_unref(xfer_op->task);
>  
>      /* SpiceFileTransferTask itself is freed after it emits "finish" */
>      g_hash_table_unref(xfer_op->xfer_task);
> diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
> index 23aab62..90c2a5d 100644
> --- a/src/spice-file-transfer-task.c
> +++ b/src/spice-file-transfer-task.c
> @@ -118,9 +118,11 @@ static void spice_file_transfer_task_query_info_cb(GObject *obj,
>      if (self->error) {
>          /* Return error previously reported */
>          g_task_return_error(task, self->error);
> +        g_object_unref(task);
>          return;
>      } else if (error) {
>          g_task_return_error(task, error);
> +        g_object_unref(task);
>          return;
>      }
>  
> @@ -132,6 +134,7 @@ static void spice_file_transfer_task_query_info_cb(GObject *obj,
>      g_object_notify(G_OBJECT(self), "progress");
>  
>      g_task_return_pointer(task, info, g_object_unref);
> +    g_object_unref(task);
>  }
>  
>  static void spice_file_transfer_task_read_file_cb(GObject *obj,
> @@ -152,10 +155,12 @@ static void spice_file_transfer_task_read_file_cb(GObject *obj,
>          /* Return error previously reported */
>          self->pending = FALSE;
>          g_task_return_error(task, self->error);
> +        g_object_unref(task);
>          return;
>      } else if (error) {
>          self->pending = FALSE;
>          g_task_return_error(task, error);
> +        g_object_unref(task);
>          return;
>      }
>  
> @@ -187,9 +192,11 @@ static void spice_file_transfer_task_read_stream_cb(GObject *source_object,
>      if (self->error) {
>          /* On any pending error on SpiceFileTransferTask */
>          g_task_return_error(task, self->error);
> +        g_object_unref(task);
>          return;
>      } else if (error) {
>          g_task_return_error(task, error);
> +        g_object_unref(task);
>          return;
>      }
>  
> @@ -209,6 +216,7 @@ static void spice_file_transfer_task_read_stream_cb(GObject *source_object,
>      }
>  
>      g_task_return_int(task, nbytes);
> +    g_object_unref(task);
>  }
>  
>  /* main context */
> @@ -433,6 +441,7 @@ void spice_file_transfer_task_read_async(SpiceFileTransferTask *self,
>           * reach a state where agent says file-transfer SUCCEED but we are in a
>           * PENDING state in SpiceFileTransferTask due reading in idle */
>          g_task_return_int(task, 0);
> +        g_object_unref(task);
>          return;
>      }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel