[Spice-devel] [spice-gtk v2 7/9] file-transfer: increase reference for channel-main

[Victor Toso]

channel-main uses the SpiceFileTransferTask reference given in the
hash table provided by spice_file_transfer_task_create_tasks(). That
means SpiceFileTransferTask is not holding a reference for itself but
it relies on it due the async calls it provides.

This patch increases the reference of each SpiceFileTransferTask for
the hash table which will be unref'ed by channel-main in
file_transfer_operation_task_finished(); the original reference is
kept to SpiceFileTransferTask to be freed after the finish signal is
emitted on spice_file_transfer_task_close_stream_cb().

We are changing the GHashTable creation using g_hash_table_full() with
g_object_unref() to simplify the usage of this GHashTable.

This patch fixes some critical warnings as we have two g_object_unref
but only one reference.
---
 src/channel-main.c             |  4 ----
 src/spice-file-transfer-task.c | 11 ++++++-----
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 5af73b4..bc7349f 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -2889,8 +2889,6 @@ static void file_transfer_operation_free(FileTransferOperation *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);
 
     spice_debug("Freeing file-transfer-operation %p", xfer_op);
@@ -2961,7 +2959,6 @@ static void file_transfer_operation_task_finished(SpiceFileTransferTask *xfer_ta
         /* Likely the operation has ended before the remove-task was called. One
          * situation that this can easily happen is if the agent is disconnected
          * while there are pending files. */
-        g_object_unref(xfer_task);
         return;
     }
 
@@ -2984,7 +2981,6 @@ static void file_transfer_operation_task_finished(SpiceFileTransferTask *xfer_ta
 
     /* Remove and free SpiceFileTransferTask */
     g_hash_table_remove(xfer_op->xfer_task, GUINT_TO_POINTER(task_id));
-    g_object_unref(xfer_task);
 
     /* Keep file_xfer_tasks up to date. If no more elements, operation is over */
     g_hash_table_remove(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(task_id));
diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index a2b3885..eb943c4 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -304,6 +304,7 @@ void spice_file_transfer_task_completed(SpiceFileTransferTask *self,
     self->pending = TRUE;
 signal:
     g_signal_emit(self, task_signals[SIGNAL_FINISHED], 0, self->error);
+    /* SpiceFileTransferTask unref is done after input stream is closed */
 }
 
 G_GNUC_INTERNAL
@@ -343,9 +344,9 @@ guint64 spice_file_transfer_task_get_bytes_read(SpiceFileTransferTask *self)
 
 /* Helper function which only creates a SpiceFileTransferTask per GFile
  * in @files and returns a HashTable mapping task-id to the task itself
- * Note that the HashTable does not free its values upon destruction:
- * The SpiceFileTransferTask reference created here should be freed by
- * spice_file_transfer_task_completed */
+ * The SpiceFileTransferTask created here has two references, one should be
+ * freed by spice_file_transfer_task_completed and the other upon GHashTable
+ * destruction */
 G_GNUC_INTERNAL
 GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
                                                   SpiceMainChannel *channel,
@@ -357,7 +358,7 @@ GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
 
     g_return_val_if_fail(files != NULL && files[0] != NULL, NULL);
 
-    xfer_ht = g_hash_table_new(g_direct_hash, g_direct_equal);
+    xfer_ht = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_object_unref);
     for (i = 0; files[i] != NULL && !g_cancellable_is_cancelled(cancellable); i++) {
         SpiceFileTransferTask *xfer_task;
         guint32 task_id;
@@ -374,7 +375,7 @@ GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
         xfer_task->flags = flags;
 
         task_id = spice_file_transfer_task_get_id(xfer_task);
-        g_hash_table_insert(xfer_ht, GUINT_TO_POINTER(task_id), xfer_task);
+        g_hash_table_insert(xfer_ht, GUINT_TO_POINTER(task_id), g_object_ref(xfer_task));
 
         /* if we created a per-task cancellable above, unref it */
         if (!cancellable)
-- 
2.7.4