summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJonathon Jongsma <jjongsma@redhat.com>2015-11-05 12:17:24 -0600
committerJonathon Jongsma <jjongsma@redhat.com>2015-11-10 10:15:11 -0600
commit401275b10abcf6c9fbbc426b40629eac2202044f (patch)
treed37679dc1058b11eaf6a4430a9cda7e8149de820
parent5bcbc4f2ba2f9e7cbe4264188257308294dda178 (diff)
downloadspice-gtk-401275b10abcf6c9fbbc426b40629eac2202044f.tar.gz
spice-gtk-401275b10abcf6c9fbbc426b40629eac2202044f.tar.xz
spice-gtk-401275b10abcf6c9fbbc426b40629eac2202044f.zip
file xfer: Fix segfault when rebooting
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@gmail.com>. 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)
-rw-r--r--src/channel-main.c11
1 files changed, 10 insertions, 1 deletions
diff --git a/src/channel-main.c b/src/channel-main.c
index 0eb40b9..8138fd5 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -3041,6 +3041,14 @@ 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_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 +3082,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));
task->priv->pending = TRUE;
/* if we created a per-task cancellable above, free it */