<div dir="ltr">Hi<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 26, 2015 at 3:35 PM, Victor Toso <span dir="ltr"><<a href="mailto:victortoso@redhat.com" target="_blank">victortoso@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>PipeInputStream and PipeOutputStream should not fail when creating<br>
GPollableStream source. It is already checked and unref in case of<br>
existing source.<br>
<br>
</span>In order to track possible issues, the g_return_val_if_fail was<br>
changed to a g_debug message;<br>
---<br>
 gtk/giopipe.c | 14 ++++++++------<br>
 1 file changed, 8 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/gtk/giopipe.c b/gtk/giopipe.c<br>
index 50edb5b..86eaab6 100644<br>
--- a/gtk/giopipe.c<br>
+++ b/gtk/giopipe.c<br>
@@ -234,10 +234,11 @@ pipe_input_stream_create_source (GPollableInputStream *stream,<br>
<span>     PipeInputStream *self = PIPE_INPUT_STREAM(stream);<br>
     GSource *pollable_source;<br>
<br>
-    g_return_val_if_fail (self->source == NULL ||<br>
-                          g_source_is_destroyed (self->source), NULL);<br>
</span>+    if (self->source != NULL && !g_source_is_destroyed (self->source))<br>
+        g_debug ("%s: GPollableSource already exists %p - This could lead to data loss (%ld)",<br>
+                 G_STRFUNC, self->source, self->read);<br>
<span><br>
-    if (self->source && g_source_is_destroyed (self->source))<br>
</span>+    if (self->source)<br>
         g_source_unref (self->source);<br>
<br>
     pollable_source = g_pollable_source_new_full (self, NULL, cancellable);<br>
@@ -416,10 +417,11 @@ pipe_output_stream_create_source (GPollableOutputStream *stream,<br>
<span>     PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);<br>
     GSource *pollable_source;<br>
<br>
-    g_return_val_if_fail (self->source == NULL ||<br>
-                          g_source_is_destroyed (self->source), NULL);<br>
</span>+    if (self->source != NULL && !g_source_is_destroyed (self->source))<br>
+        g_debug ("%s: GPollableSource already exists %p - This could lead to data loss (%ld)",<br>
+                 G_STRFUNC, self->source, self->count);<br>
<span><br>
-    if (self->source && g_source_is_destroyed (self->source))<br>
</span>+    if (self->source)<br>
         g_source_unref (self->source);<br>
<br>
     pollable_source = g_pollable_source_new_full (self, NULL, cancellable);<br>
<div><div>--<br></div></div></blockquote><div><br></div><div>Your tests show that io stream prevents concurrent read / write, and that self->source is going to be destroyed after the current tasks return. However, if gpollable create_source is called seperately, it may create "zombie" sources, they will never be dispatched. It is easy to show the issue with a test like:<br clear="all"></div></div><br>    for (i = 0; i < 10000; i++) {<br>        GSource *s = g_pollable_input_stream_create_source(f->ip1, NULL);<br>     <br>        g_source_set_callback (s, NULL, NULL, NULL);<br>        g_source_attach (s, NULL);<br>    <br>        g_source_unref(s);<br>    }<br><br>    g_main_loop_run (f->loop);<br><br></div><div class="gmail_extra">After the source is attached, the context will keep a reference, but when creating a new source, it is not removed from the context. giopipe could also call g_source_destroy() before the unref, but I don't think this is a good practice: destroyed source callbacks would never be called, and it may be hard to understand why.<br><br>I suppose the best would be to mimic poll() behaviour, and "dispatch" with set_ready_time(0) all the created sources (that are still active).<br><br>-- <br><div>Marc-André Lureau</div>
</div></div></div>