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

[Christophe Fergeau]

On Tue, Feb 07, 2017 at 06:03:53PM +0100, Christophe Fergeau wrote:
> On Tue, Jan 17, 2017 at 03:29:41PM +0100, Debarshi Ray wrote:
> > From: Debarshi Ray <debarshir at gnome.org>
> > 
> > Even though g_main_loop_new accepts a is_running parameter, it isn't
> > very important since g_main_loop_run will set it to TRUE anyway. There
> > is no requirement that it should be set before calling g_main_loop_run.
> > The vast majority of GMainLoop users simply ignore the is_running
> > parameter unless they are doing something out of the ordinary.
> 
> 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?

Looks like this could cause problems if g_main_loop_quit() can be called
before the main loop starts running, I tested with this:

#include <glib.h>

static gboolean assert_not_reached(gpointer user_data)
{
    g_assert_not_reached();
}

int main(int argc, char **argv)
{
    GMainLoop *loop;

    g_print("first loop test\n");

    loop = g_main_loop_new(NULL, TRUE);
    g_main_loop_quit(loop);
    g_timeout_add_seconds(1, assert_not_reached, NULL);
    if (g_main_loop_is_running(loop)) {
        g_main_loop_run(loop);
    }
    g_main_loop_unref(loop);


    g_print("second loop test\n");

    loop = g_main_loop_new(NULL, FALSE);
    g_main_loop_quit(loop);
    g_timeout_add_seconds(1, assert_not_reached, NULL);
    g_main_loop_run(loop);
    g_main_loop_unref(loop);

    return 0;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libosinfo/attachments/20170208/406bfd4b/attachment.sig>