[Libosinfo] [PATCH] Marginally simplify the code to create and run a GMainLoop

[Debarshi Ray]

Hey,

Sorry for the late response.

On Tue, Feb 07, 2017 at 06:03:53PM +0100, Christophe Fergeau wrote:
> Hey, looking for example at osinfo_install_script_generate(), which is
> 
>     GMainLoop *loop = g_main_loop_new(g_main_context_get_thread_default(),
>                                       TRUE);
>     OsinfoInstallScriptGenerateSyncData data = {
>         loop, NULL, NULL, NULL
>     };
> 
>     osinfo_install_script_generate_async(script,
>                                          os,
>                                          config,
>                                          cancellable,
>                                          osinfo_install_script_generate_done,
>                                          &data);
> 
>     if (g_main_loop_is_running(loop))
>         g_main_loop_run(loop);
> 
> Isn't the way it's currently done going to catch cases when
> osinfo_install_script_generate_done (and thus g_main_loop_quit()) ends up
> being called synchronously at the moment
> osinfo_install_script_generate_async() is called, while with your change,
> we'd wait forever for a g_main_loop_quit() which never comes?

Generally, I wouldn't expect an async operation to invoke the callback
in the same iteration of the GMainLoop. Otherwise, it will be
confusing for the user code. It wouldn't know if the async operation
will actually proceed to some extent and then return in a later
iteration of the GMainLoop, or whether it would take a short cut and
return immediately.

I don't see it clearly specified in the semantics of GAsyncResult
interface, but the GTask implmentation used in libosinfo guarantees
this. See the private g_task_return in gio/gtask.c.

The earlier GSimpleAsyncResult was a bit lax in this area, but if you
look closely, the g_simple_async_report_* methods also invoked the
callback in the next iteration of the GMainLoop. One of the
motivations to return immediately are error conditions detected at the
beginning, which is what those methods usually deal with.

By the way, this patch is just a clean-up and hence not that
important.  The real bug in libosinfo is that the synchronous
operations implemented in terms of their async counterparts are not
wrapped in a new thread-default GMainContext. Since the GMainLoop is
iterating over the thread's existing default GMainContext, it can
trigger GSources outside the scope of the synchronous method.

Cheers,
Rishi