[libvirt] [libvirt-glib 3/3] gobject: Port to GTask API

[Christophe Fergeau]

On Fri, Nov 20, 2015 at 03:57:08PM -0500, Zeeshan Ali (Khattak) wrote:
> >> diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c
> >> index 46dbd9a..f8a1a8c 100644
> >> --- a/libvirt-gobject/libvirt-gobject-stream.c
> >> +++ b/libvirt-gobject/libvirt-gobject-stream.c
> >> @@ -129,14 +129,13 @@ static gboolean gvir_stream_close(GIOStream *io_stream,
> >>      return (i_ret && o_ret);
> >>  }
> >>
> >> -
> >> -static void gvir_stream_close_async(GIOStream *stream,
> >> -                                    int io_priority G_GNUC_UNUSED,
> >> -                                    GCancellable *cancellable,
> >> -                                    GAsyncReadyCallback callback,
> >> -                                    gpointer user_data)
> >> +static void
> >> +gvir_stream_close_helper(GTask *task,
> >> +                         gpointer source_object,
> >> +                         gpointer task_data G_GNUC_UNUSED,
> >> +                         GCancellable *cancellable)
> >>  {
> >> -    GSimpleAsyncResult *res;
> >> +    GIOStream *stream = G_IO_STREAM(source_object);
> >>      GIOStreamClass *class;
> >>      GError *error;
> >>
> >> @@ -146,27 +145,36 @@ static void gvir_stream_close_async(GIOStream *stream,
> >>      error = NULL;
> >>      if (class->close_fn &&
> >>          !class->close_fn(stream, cancellable, &error)) {
> >> -        g_simple_async_report_take_gerror_in_idle(G_OBJECT (stream),
> >> -                                                  callback, user_data,
> >> -                                                  error);
> >> +        g_task_return_error(task, error);
> >>          return;
> >>      }
> >>
> >> -    res = g_simple_async_result_new(G_OBJECT (stream),
> >> -                                    callback,
> >> -                                    user_data,
> >> -                                    gvir_stream_close_async);
> >> -    g_simple_async_result_complete_in_idle(res);
> >> -    g_object_unref (res);
> >> +    g_task_return_boolean(task, TRUE);
> >> +}
> >> +
> >> +static void gvir_stream_close_async(GIOStream *stream,
> >> +                                    int io_priority G_GNUC_UNUSED,
> >> +                                    GCancellable *cancellable,
> >> +                                    GAsyncReadyCallback callback,
> >> +                                    gpointer user_data)
> >> +{
> >> +    GTask *task;
> >> +
> >> +    task = g_task_new(G_OBJECT(stream),
> >> +                      cancellable,
> >> +                      callback,
> >> +                      user_data);
> >> +    g_task_run_in_thread(task, gvir_stream_close_helper);
> >> +    g_object_unref(task);
> >>  }
> >
> > Was this one creating a GSimpleAsyncResult and returning immediately
> > without doing anything?
> 
> I doubt so. AFAICT, it was scheduling the result to be returned in the
> idle using 'g_simple_async_result_complete_in_idle'.

Sorry, I  was not explicit, but this is what I meant by "returning
immediately", as opposed to creating a thread to handle the call (as all
the other methods).

> 
> > The helper does not seem to be doing anything more, in which case I'd
> > suggest not creating an intermediate helper thread, and to do something
> > similar to what was done before. If this is a bug and more work should
> > be done, I'd prefer if this was done in a separate patch.
> 
> I just failed to find a straight GTask replacement API for
> 'g_simple_async_result_complete_in_idle'. The docs for
> 'g_task_return_pointer' say:
> 
> ""Completes the task" means that for an ordinary asynchronous task it
> will either invoke the task's callback, or else queue that callback to
> be invoked in the proper GMainContext, or in the next iteration of the
> current GMainContext."
> 
> So I'm not sure if simply calling g_task_return_*() wouldn't change
> the behaviour and call the callback immediately.

For me, the main point of _complete_in_idle() was to make sure that the
callback gets called in the main thread rather than in a random thread.
Since g_task_return_*() handles this properly, there is indeed no
_in_idle() variant.

In my opinion, it's no big deal whether the callback gets run from an
idle or directly as long as this happens in the 'right' thread. If you
want to keep the 'run in idle' behaviour, I guess you could run
g_task_return_*() in a g_idle_add() callback, and avoid the spawning of
a thread.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151123/a1c62577/attachment-0001.sig>