[Spice-commits] 2 commits - src/channel-main.c src/spice-file-transfer-task.c

[Victor Toso de Carvalho]

 src/channel-main.c             |    4 ----
 src/spice-file-transfer-task.c |   12 +++++++-----
 2 files changed, 7 insertions(+), 9 deletions(-)

New commits:
commit 5f0df31aec24f9eb5e829a5b367038f4b0ef9850
Author: Victor Toso <victortoso at redhat.com>
Date:   Wed Aug 3 16:16:01 2016 +0200

    file-transfer: improve GHashTable with value_destroy_func
    
    By using g_hash_table_new_full() we can unref its value which is the
    SpiceFileTransferTask. This makes the code a little bit simpler as
    nowhere we use the xfer-task after removing it from hash table.
    
    Acked-by: Pavel Grunt <pgrunt at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index 47e19e1..a074928 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -2898,8 +2898,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);
@@ -2970,7 +2968,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;
     }
 
@@ -2993,7 +2990,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 58340e3..e7f50da 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -380,7 +380,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;
commit 27756bae62d6e6d9a64c396d7d21cf3c084130be
Author: Victor Toso <victortoso at redhat.com>
Date:   Fri Jul 29 18:08:45 2016 +0200

    file-transfer: increase reference for channel-main
    
    SpiceMainChannel 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().
    
    This patch fixes some critical warnings as we have two g_object_unref
    but only one reference.
    
    Acked-by: Pavel Grunt <pgrunt at redhat.com>

diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index 4689b71..58340e3 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -325,6 +325,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
@@ -364,9 +365,10 @@ 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_close_stream_cb() after
+ * spice_file_transfer_task_completed() is called and the other reference
+ * belongs to the caller and should be freed upon GHashTable destruction */
 G_GNUC_INTERNAL
 GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
                                                   SpiceMainChannel *channel,
@@ -385,7 +387,7 @@ GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
 
         xfer_task = spice_file_transfer_task_new(channel, files[i], flags, cancellable);
         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));
     }
     return xfer_ht;
 }