[Spice-devel] [spice-gtk v1 06/10] file-transfer: Fix SpiceFileTransferTask::error leak

[Christophe Fergeau]

On Sat, Jul 30, 2016 at 12:26:27AM +0200, Victor Toso wrote:
> The self->error is the error set for the file-transfer and it will be
> propagated with the "finish" signal. As this is transfer none pointer,
> we should clear it out afterwards.
> 
> 40 (16 direct, 24 indirect) bytes in 1 blocks are definitely lost in
> loss record 7,489 of 14,298
>  at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
>  by 0xB5090E8: g_malloc (gmem.c:94)
>  by 0xB51F8A2: g_slice_alloc (gslice.c:1025)
>  by 0xB4EFCC5: g_error_new_literal (gerror.c:471)
>  by 0xB4EFFAD: g_set_error_literal (gerror.c:619)
>  by 0xAF13397: g_cancellable_set_error_if_cancelled (gcancellable.c:314)
>  by 0xAF630C8: g_task_propagate_error (gtask.c:1519)
>  by 0xAF63CD8: g_task_propagate_int (gtask.c:1652)
>  by 0x50863F5: spice_file_transfer_task_read_finish (spice-file-transfer-task.c:477)
>  by 0x5093239: file_xfer_read_async_cb (channel-main.c:1811)
>  by 0xAF62F38: g_task_return_now (gtask.c:1121)
>  by 0xAF63775: g_task_return (gtask.c:1179)
> ---
>  src/spice-file-transfer-task.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
> index ea46c9d..667d230 100644
> --- a/src/spice-file-transfer-task.c
> +++ b/src/spice-file-transfer-task.c
> @@ -589,6 +589,7 @@ spice_file_transfer_task_dispose(GObject *object)
>  
>      g_clear_object(&self->file);
>      g_clear_object(&self->file_stream);
> +    g_clear_error(&self->error);
>  
>      G_OBJECT_CLASS(spice_file_transfer_task_parent_class)->dispose(object);

I think you'll need at least something like
diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index 70571f0..ba99e1f 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -122,6 +122,7 @@ static void spice_file_transfer_task_query_info_cb(GObject *obj,
         g_clear_error(&error);
         /* Return error previously reported */
         g_task_return_error(task, self->error);
+        self->error = NULL;
         g_object_unref(task);
         return;
     } else if (error) {
@@ -160,6 +161,7 @@ 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);
+        self->error = NULL;
         g_object_unref(task);
         return;
     } else if (error) {
@@ -198,6 +200,7 @@ static void spice_file_transfer_task_read_stream_cb(GObject *source_object,
         g_clear_error(&error);
         /* On any pending error on SpiceFileTransferTask */
         g_task_return_error(task, self->error);
+        self->error = NULL;
         g_object_unref(task);
         return;
     } else if (error) {


(or to g_error_copy() the error you pass to g_task_return_error()).
Though I must admit I haven't been able to easily follow the codepaths handling self->error,
so maybe I missed something.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160801/a183704c/attachment.sig>