[Spice-devel] [spice-gtk v1] file-xfer: avoid g_hash_table_iter_* when changing the GHashTable

[Pavel Grunt]

On Wed, 2017-02-08 at 10:26 +0100, Victor Toso wrote:
> Hi,
> 
> On Wed, Feb 08, 2017 at 10:25:10AM +0100, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> > 
> > Using g_hash_table_iter_init() and g_hash_table_iter_next() here
> > is
> > bad as spice_file_transfer_task_completed() will emit "finished"
> > signal from SpiceFileTransferTask resulting in the original
> > GHashTable
> > to be changed in file_transfer_operation_task_finished()
> > 
> > Debug will show:
> >   GSpice-DEBUG: spice-file-transfer-task.c:303 File bigfile2 xfer
> >   failed: Agent connection closed
> > 
> >   WARNING **: Agent connection closed
> > 
> >   GLib-CRITICAL **: g_hash_table_iter_next: assertion 'ri->version 
> > ==
> >   ri->hash_table->version' failed
> > 
> > Reported-by: Pavel Grunt <pgrunt at redhat.com>
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/channel-main.c | 18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index ed5d611..7e7dd28 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -2915,19 +2915,23 @@ static void
> > file_transfer_operation_free(FileTransferOperation *xfer_op)
> >  
> >  static void
> > spice_main_channel_reset_all_xfer_operations(SpiceMainChannel
> > *channel)
> >  {
> > -    GHashTableIter iter_all_xfer_tasks;
> > -    gpointer key, value;
> > +    GList *it, *keys;
> >  
> >      /* Mark each of SpiceFileTransferTask as completed due error
> > */
> > -    g_hash_table_iter_init(&iter_all_xfer_tasks, channel->priv-
> > >file_xfer_tasks);
> > -    while (g_hash_table_iter_next(&iter_all_xfer_tasks, &key,
> > &value)) {
> > -        FileTransferOperation *xfer_op = value;
> > -        SpiceFileTransferTask *xfer_task =
> > g_hash_table_lookup(xfer_op->xfer_task, key);
> > +    keys = g_hash_table_get_keys(channel->priv->file_xfer_tasks);
> 
> I forgot to g_list_free() it afterwards, fixed locally

ack

Thanks,
Pavel
> 
> 
> > +    for (it = keys; it != NULL; it = it->next) {
> > +        FileTransferOperation *xfer_op;
> > +        SpiceFileTransferTask *xfer_task;
> >          GError *error;
> >  
> > +        xfer_op = g_hash_table_lookup(channel->priv-
> > >file_xfer_tasks, it->data);
> > +        if (xfer_op == NULL)
> > +            continue;
> > +
> > +        xfer_task = g_hash_table_lookup(xfer_op->xfer_task, it-
> > >data);
> >          if (xfer_task == NULL) {
> >              spice_warning("(reset-all) can't complete task %u -
> > completed already?",
> > -                          GPOINTER_TO_UINT(key));
> > +                          GPOINTER_TO_UINT(it->data));
> >              continue;
> >          }
> >  
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel