[Spice-devel] [spice-gtk PATCH v5 1/6] giopipe: don't fail on create_source

[Marc-André Lureau]

----- Original Message -----
> PipeInputStream and PipeOutputStream should not fail when creating
> GPollableStream source as this currently does not work with default
> write_all and read_all functions;
> 
> In order to avoid creating zombie GSource in create_source of both
> PipeInputStream and PipeOutputStream, we track all created GSources and
> set them to be dispatched when data is available to read/write. It is
> worth to mention that concurrent write/read is not possible with current
> giopipe and only the last created GSource will read the data as it is
> dispatched first.
> ---
>  gtk/giopipe.c | 72
>  ++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/gtk/giopipe.c b/gtk/giopipe.c
> index 50edb5b..699428d 100644
> --- a/gtk/giopipe.c
> +++ b/gtk/giopipe.c
> @@ -44,7 +44,7 @@ struct _PipeInputStream
>       * closing.
>       */
>      gboolean peer_closed;
> -    GSource *source;
> +    GList *sources;
>  };
>  
>  struct _PipeInputStreamClass
> @@ -69,7 +69,7 @@ struct _PipeOutputStream
>      const gchar *buffer;
>      gsize count;
>      gboolean peer_closed;
> -    GSource *source;
> +    GList *sources;
>  };
>  
>  struct _PipeOutputStreamClass
> @@ -120,12 +120,39 @@ pipe_input_stream_read (GInputStream  *stream,
>      return count;
>  }
>  
> +static GList *
> +set_all_sources_ready (GList *sources)
> +{
> +    GList *it = sources;
> +    while (it != NULL) {
> +        GSource *s = it->data;
> +        GList *next = it->next;
> +
> +        if (s == NULL || g_source_is_destroyed(s)) {
> +            /* remove */
> +            sources = g_list_delete_link(sources, it);
> +            g_clear_pointer(&s, g_source_unref);
> +        } else {
> +            /* dispatch */
> +            g_source_set_ready_time(s, 0);
> +        }
> +        it = next;
> +    }
> +    return sources;
> +}
> +
>  static void
>  pipe_input_stream_check_source (PipeInputStream *self)
>  {
> -    if (self->source && !g_source_is_destroyed(self->source) &&
> +    GSource *source;
> +
> +    if (self->sources == NULL)
> +        return;
> +
> +    source = self->sources->data;
> +    if (source && !g_source_is_destroyed(source) &&
>          g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
> -        g_source_set_ready_time(self->source, 0);
> +        self->sources = set_all_sources_ready(self->sources);

No idea why you check the first source here. I think the function could be just like:

if (g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(self)))
   self->sources = set_all_sources_ready(self->sources)
    
>  }
>  
>  static gboolean
> @@ -193,10 +220,8 @@ pipe_input_stream_dispose(GObject *object)
>          self->peer = NULL;
>      }
>  
> -    if (self->source) {
> -        g_source_unref(self->source);
> -        self->source = NULL;
> -    }
> +    g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
> +    self->sources = NULL;
>  
>      G_OBJECT_CLASS(pipe_input_stream_parent_class)->dispose (object);
>  }
> @@ -234,14 +259,8 @@ pipe_input_stream_create_source (GPollableInputStream
> *stream,
>      PipeInputStream *self = PIPE_INPUT_STREAM(stream);
>      GSource *pollable_source;
>  
> -    g_return_val_if_fail (self->source == NULL ||
> -                          g_source_is_destroyed (self->source), NULL);
> -
> -    if (self->source && g_source_is_destroyed (self->source))
> -        g_source_unref (self->source);
> -
>      pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
> -    self->source = g_source_ref (pollable_source);
> +    self->sources = g_list_prepend (self->sources, g_source_ref
> (pollable_source));
>  
>      return pollable_source;
>  }
> @@ -319,10 +338,8 @@ pipe_output_stream_dispose(GObject *object)
>          self->peer = NULL;
>      }
>  
> -    if (self->source) {
> -        g_source_unref(self->source);
> -        self->source = NULL;
> -    }
> +    g_list_free_full (self->sources, (GDestroyNotify) g_source_unref);
> +    self->sources = NULL;
>  
>      G_OBJECT_CLASS(pipe_output_stream_parent_class)->dispose (object);
>  }
> @@ -330,9 +347,14 @@ pipe_output_stream_dispose(GObject *object)
>  static void
>  pipe_output_stream_check_source (PipeOutputStream *self)
>  {
> -    if (self->source && !g_source_is_destroyed(self->source) &&
> +    GSource * source;
> +    if (self->sources == NULL)
> +        return;
> +
> +    source = self->sources->data;
> +    if (source && !g_source_is_destroyed(source) &&
>          g_pollable_output_stream_is_writable(G_POLLABLE_OUTPUT_STREAM(self)))
> -        g_source_set_ready_time(self->source, 0);
> +        self->sources = set_all_sources_ready(self->sources);

same here

>  }
>  
>  static gboolean
> @@ -416,14 +438,8 @@ pipe_output_stream_create_source (GPollableOutputStream
> *stream,
>      PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
>      GSource *pollable_source;
>  
> -    g_return_val_if_fail (self->source == NULL ||
> -                          g_source_is_destroyed (self->source), NULL);
> -
> -    if (self->source && g_source_is_destroyed (self->source))
> -        g_source_unref (self->source);
> -
>      pollable_source = g_pollable_source_new_full (self, NULL, cancellable);
> -    self->source = g_source_ref (pollable_source);
> +    self->sources = g_list_prepend (self->sources, g_source_ref
> (pollable_source));
>  
>      return pollable_source;
>  }
> --
> 2.4.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>