----- Original Message -----
> Recent changes to file transfer introduced a regression where the client
> would crash when rebooting a guest after performing a file transfer.
> This was caused because the SpiceFileTransferTask is freed when it is
> completed, but is not removed from the MainChannel's hash table. When we
> reboot the guest and lose our vdagent connection, we iterate through the
> list of tasks in the hash table and complete them. But since we did not
> remove the already-completed tasks from this hash table, this hash table
> contains already-freed memory.
>
> To fix the issue, take an extra ref for the async operations (so that
> completing the task won't free an object that is stored in the hash table).
> In
> addition, connect to the task's "finished" signal and remove it from the hash
> table when it becomes finished.
>
> Bug reported via email by Jay.han <ezzzehxx@xxxxxxxxx>. Valgrind report
> below:
>
> ==6926== Invalid read of size 8
> ==6926== at 0x508177B: spice_file_transfer_task_completed
> (channel-main.c:2941)
> ==6926== by 0x50846DC: set_agent_connected (channel-main.c:462)
> ==6926== by 0x5073A43: spice_channel_recv_msg (spice-channel.c:1892)
> ==6926== by 0x5073BE3: spice_channel_iterate_read (spice-channel.c:2132)
> ==6926== by 0x5075D25: spice_channel_coroutine (spice-channel.c:2170)
> ==6926== by 0x50A6EFE: coroutine_trampoline (coroutine_ucontext.c:63)
> ==6926== by 0x50A6CC8: continuation_trampoline (continuation.c:55)
> ==6926== by 0x65C2B5F: ??? (in /lib/x86_64-linux-gnu/libc-2.19.so)
> ==6926== by 0x151331C7: ???
> ==6926== Address 0x29971fd8 is 168 bytes inside a block of size 184 free'd
> ==6926== at 0x4C2BDEC: free (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6926== by 0x5E33142: g_type_free_instance (in
> /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.4000.0)
> ==6926== by 0x50815DA: file_xfer_close_cb (channel-main.c:1826)
> ==6926== by 0x5AEBD5C: ??? (in
> /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
> ==6926== by 0x5B0F41A: ??? (in
> /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
> ==6926== by 0x5B0F438: ??? (in
> /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
> ==6926== by 0x609BCE4: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==6926== by 0x609C047: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==6926== by 0x609C309: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==6926== by 0x4058AB: main (spicy.c:1858)
> ---
> src/channel-main.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 0eb40b9..5abb32d 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -3041,6 +3041,16 @@ static SpiceFileTransferTask
> *spice_file_transfer_task_new(SpiceMainChannel *cha
> GFile *file,
> GCancellable
> *cancellable);
>
> +static void task_finished(SpiceFileTransferTask *task,
> + GError *error,
> + gpointer data)
> +{
> + SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(data);
> + g_debug("%s: %i", G_STRFUNC, task->priv->id);
> +
> + g_hash_table_remove(channel->priv->file_xfer_tasks,
> GUINT_TO_POINTER(task->priv->id));
> +}
> +
> static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
> GFile **files,
> GFileCopyFlags flags,
> @@ -3074,13 +3084,14 @@ static void
> file_xfer_send_start_msg_async(SpiceMainChannel *channel,
> g_hash_table_insert(c->file_xfer_tasks,
> GUINT_TO_POINTER(task->priv->id),
> task);
> + g_signal_connect(task, "finished", G_CALLBACK(task_finished),
> channel);
> g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0,
> task);
>
> g_file_read_async(files[i],
> G_PRIORITY_DEFAULT,
> cancellable,
> file_xfer_read_async_cb,
> - task);
> + g_object_ref(task));
This looks like it will leak. There is now 2 refs, right? where are the corresponding 2 unrefs? I count one with the hash table.
> task->priv->pending = TRUE;
>
> /* if we created a per-task cancellable above, free it */
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel