[Spice-devel] [spice-gtk v2] Introduce gtask-helper.[ch]

[Victor Toso]

On Fri, May 13, 2016 at 03:11:31PM +0200, Christophe Fergeau wrote:
> Hey,
> 
> On Fri, May 13, 2016 at 02:30:25PM +0200, Victor Toso wrote:
> > With that said, I'd like to suggest a change in the gtask-helper.
> > Quoting the fine manual from GTask:
> >  "The "return" methods (eg, g_task_return_pointer()) automatically cause
> >  the task to be "completed" as well, and there is no need to worry about
> >  the "complete" vs "complete in idle" distinction. (GTask automatically
> >  figures out whether the task's callback can be invoked directly, or if
> >  it needs to be sent to another GMainContext, or delayed until the next
> >  iteration of the current GMainContext.)"
> > 
> > I would like that gtaskhelper do what the GTask can't at this moment
> > with the coroutine context which is "not make us worry about complete vs
> > complete in idle"
> > 
> > I would like to know what you think about changing
> > g_task_helper_return_int_in_idle to g_task_helper_return_int. The
> > non-idle version should then check if we are in the main-context using
> > coroutine_self_is_main(). If we are in the main-context, I would say we
> > can let g_task_return_int decide when to call the callback. If we are in
> > coroutine context, we can return in idle.
> > 
> > This approach would let us be closer to the gtask behavior/intention; we
> > would only be handling the coroutine context that gtask can't at the
> > moment.
>
> I don't think I agree with this approach. Hiding it in gtask-helper
> would mean that we think that all our GTask callbacks happening in a coroutine
> context need to happen in a main context.

Right. If we have a GTask created under coroutine context, you are 100%
right. I don't remember any, but I'll double check.
>
> What you could do is record in which coroutine context you are at GTask
> creation, and compare to the context we are in when returning from the
> coroutine, but that adds even more complication...

That's seems good, not sure if it is too complicated. GTask does the
same but it can't handle the coroutine context.

I'd go for your approach, record the context of GTask creation (with
coroutine_self_is_main()). The annoying part would be the callback
under coroutine context.

> You would also need more magic to guarantee a coroutine switch before
> the callback is called in an idle (or are all idles called in the main
> context? I forgot this). In this case, my understanding is that the code
> was working before because something between the
> g_simple_async_result_return_in_idle and the invocation of the idle
> callback was triggering a coroutine switch.
>
> Christophe

AFAIK, all callbacks with g_simple_async_result_return_in_idle were
expecting to be under main context. We don't need to trigger a
coroutine switch.

  toso