[Spice-devel] [spice-gtk (rebased) v3 1/2] file-transfer: increase reference for channel-main

[Victor Toso]

Hi,

On Wed, Aug 10, 2016 at 01:45:59PM +0200, Pavel Grunt wrote:
> Ack for both patches

Thanks pushed as
https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=27756bae62d6e6d9a64c396d7d21cf3c084130be
and
https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=5f0df31aec24f9eb5e829a5b367038f4b0ef9850

Cheers,
  toso

>
> Pavel
> 
> On Wed, 2016-08-10 at 10:06 +0200, Victor Toso wrote:
> > 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.
> > ---
> >  src/spice-file-transfer-task.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > 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;
> >  }
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel