Hi, On Wed, May 20, 2015 at 12:51:46PM +0200, Marc-André Lureau wrote: > 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). > I created the test with sequential operations because this is how it works with channel-webdav. Just to be sure, I created a test with with concurrent write and we get G_IO_ERROR_PENDING as error. So, regarding concurrent write I believe we are safe. > 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. I've applied your suggestions plus the concurrent test, sending it now. Many thanks for the explanation and review, Victor Toso _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel