Re: [spice-gtk PATCH v2 1/5] giopipe: don't fail on create_source

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Tue, May 19, 2015 at 4:34 PM, Victor Toso <victortoso@xxxxxxxxxx> wrote:
PipeInputStream and PipeOutputStream should not fail when creating
GPollableStream source. It is already checked and unref in case of
existing source.
---
 gtk/giopipe.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/gtk/giopipe.c b/gtk/giopipe.c
index 440cae9..32fa4fa 100644
--- a/gtk/giopipe.c
+++ b/gtk/giopipe.c
@@ -234,9 +234,6 @@ 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);

@@ -416,9 +413,6 @@ 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);

The check was also preventing concurrent read/write. Without this check, it is possible to have several "active" sources on the same stream. However, it will notify only the last reader/writer. With your test though, the operations are sequential, the old source is destroyed when the loop ends the dispatch(). However, to avoid leaks, the  if (self->source && g_source_is_destroyed (self->source)) check must be changed to if (self->source).

This is potentially dangerous change. I would be okay with it, but it needs a strong warning, perhaps a debug() message. Other solution is to track all sources and notify the one that are not destroyed.



--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]