[Spice-devel] [PATCH spice-gtk 11/13] coroutine: reference object when signaling

[Marc-André Lureau]

----- Original Message -----
> Hey,
> 
> On Sun, Nov 09, 2014 at 05:31:43PM +0100, Marc-André Lureau wrote:
> > Before the signal is acutally emitted, the channel may be
> 
> 'actually'
> 
> > released. Similarly to g_signal_emit, take a reference before yielding
> > to main context.
> 
> I would drop the "similarly to g_signal_emit" as your purpose is most
> likely different. The ref is needed here as the signal emission is
> async, so code can run before the signal emission, but g_signal_emit is
> synchronous.
> 
> I assume the scenario you want to protect against is
> - (coroutine context)g_coroutine_signal_emit is called
>   - the signal emission is scheduled in an idle with g_idle_add
>   - coroutine_yield() is called
> - (main context)code runs which drops the last ref on the channel
> - (main context)idle runs and tries to emit the signal
> 
> 
> Patch looks good if this is the correct scenario.

Yes, I maintain "similarly", because we want to maintain object alive during g_coroutine_signal_emit(). I think it is thus appropriate here.

> Christophe
> 
> > ---
> >  gtk/gio-coroutine.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gtk/gio-coroutine.c b/gtk/gio-coroutine.c
> > index c903bd2..c866e15 100644
> > --- a/gtk/gio-coroutine.c
> > +++ b/gtk/gio-coroutine.c
> > @@ -222,9 +222,11 @@ g_coroutine_signal_emit(gpointer instance, guint
> > signal_id,
> >      if (coroutine_self_is_main()) {
> >          g_signal_emit_valist(instance, signal_id, detail, data.var_args);
> >      } else {
> > +        g_object_ref(instance);
> >          g_idle_add(emit_main_context, &data);
> >          coroutine_yield(NULL);
> >          g_warn_if_fail(data.notified);
> > +        g_object_unref(instance);
> >      }
> >  
> >      va_end (data.var_args);
> > @@ -253,7 +255,7 @@ void g_coroutine_object_notify(GObject *object,
> >          g_object_notify(object, property_name);
> >      } else {
> >  
> > -        data.instance = object;
> > +        data.instance = g_object_ref(object);
> >          data.caller = coroutine_self();
> >          data.propname = (gpointer)property_name;
> >          data.notified = FALSE;
> > @@ -268,5 +270,6 @@ void g_coroutine_object_notify(GObject *object,
> >           */
> >          coroutine_yield(NULL);
> >          g_warn_if_fail(data.notified);
> > +        g_object_unref(object);
> >      }
> >  }
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>