[Spice-devel] [spice-gtk v1 09/10] channel-main: check if file-transfer was completed on idle

[Victor Toso]

This patch avoids a race condition. The race happens when the flush
callback is in idle when we receive a completed transfer status for
the same file-transfer which will be marked as completed and removed
from FileTransferOperations hash table.

Channel-main should check in its async callbacks if given
SpiceFileTransferTask is completed.

This patch introduces spice_file_transfer_task_is_completed (internal)
to help check if spice_file_transfer_task_completed() was called or
not.
---
 src/channel-main.c                  | 17 +++++++++++------
 src/spice-file-transfer-task-priv.h |  1 +
 src/spice-file-transfer-task.c      | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 5af73b4..d9fd025 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1766,10 +1766,12 @@ static void file_xfer_data_flushed_cb(GObject *source_object,
         return;
     }
 
-    file_transfer_operation_send_progress(xfer_task);
-
-    /* Read more data */
-    spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, NULL);
+    /* task might be completed while on idle */
+    if (!spice_file_transfer_task_is_completed(xfer_task)) {
+        file_transfer_operation_send_progress(xfer_task);
+        /* Read more data */
+        spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, NULL);
+    }
 }
 
 static void file_xfer_queue_msg_to_agent(SpiceMainChannel *channel,
@@ -1814,13 +1816,15 @@ static void file_xfer_read_async_cb(GObject *source_object,
     }
 
     file_xfer_queue_msg_to_agent(channel, spice_file_transfer_task_get_id(xfer_task), buffer, count);
-    if (count == 0) {
-        /* on EOF just wait for VD_AGENT_FILE_XFER_STATUS from agent */
+    if (count == 0 || spice_file_transfer_task_is_completed(xfer_task)) {
+        /* on EOF just wait for VD_AGENT_FILE_XFER_STATUS from agent
+         * in case the task was completed, nothing to do. */
         return;
     }
 
     task_id = spice_file_transfer_task_get_id(xfer_task);
     xfer_op = g_hash_table_lookup(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(task_id));
+    g_return_if_fail(xfer_op != NULL);
     xfer_op->stats.total_sent += count;
 
     cancellable = spice_file_transfer_task_get_cancellable(xfer_task);
@@ -1841,6 +1845,7 @@ static void main_agent_handle_xfer_status(SpiceMainChannel *channel,
 
     switch (msg->result) {
     case VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA:
+        g_return_if_fail(spice_file_transfer_task_is_completed(xfer_task) == FALSE);
         spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, NULL);
         return;
     case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
diff --git a/src/spice-file-transfer-task-priv.h b/src/spice-file-transfer-task-priv.h
index 1ceb392..a7b58d8 100644
--- a/src/spice-file-transfer-task-priv.h
+++ b/src/spice-file-transfer-task-priv.h
@@ -52,6 +52,7 @@ gssize spice_file_transfer_task_read_finish(SpiceFileTransferTask *self,
                                             GError **error);
 guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask *self);
 guint64 spice_file_transfer_task_get_bytes_read(SpiceFileTransferTask *self);
+gboolean spice_file_transfer_task_is_completed(SpiceFileTransferTask *self);
 
 G_END_DECLS
 
diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index 8ebd256..fb48b9b 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -42,6 +42,7 @@ struct _SpiceFileTransferTask
     GObject parent;
 
     uint32_t                       id;
+    gboolean                       completed;
     gboolean                       pending;
     GFile                          *file;
     SpiceMainChannel               *channel;
@@ -270,6 +271,8 @@ G_GNUC_INTERNAL
 void spice_file_transfer_task_completed(SpiceFileTransferTask *self,
                                         GError *error)
 {
+    self->completed = TRUE;
+
     /* In case of multiple errors we only report the first error */
     if (self->error)
         g_clear_error(&error);
@@ -478,6 +481,16 @@ gssize spice_file_transfer_task_read_finish(SpiceFileTransferTask *self,
     return nbytes;
 }
 
+G_GNUC_INTERNAL
+gboolean spice_file_transfer_task_is_completed(SpiceFileTransferTask *self)
+{
+    g_return_val_if_fail(self != NULL, FALSE);
+
+    /* File transfer is considered over at the first time it is marked as
+     * complete with spice_file_transfer_task_completed. */
+    return self->completed;
+}
+
 /*******************************************************************************
  * External API
  ******************************************************************************/
@@ -735,4 +748,5 @@ static void
 spice_file_transfer_task_init(SpiceFileTransferTask *self)
 {
     self->buffer = g_malloc0(FILE_XFER_CHUNK_SIZE);
+    self->completed = FALSE;
 }
-- 
2.7.4