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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey,

On Wed, Feb 08, 2017 at 11:16:20AM +0100, Christophe Fergeau wrote:
> 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@xxxxxxxxx>
> > > 
> > > 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:

Just took another look at it, and we don't have this situation currently
in libosinfo codebase, so your patch should be fine.

Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

Have you sent the follow-up that you mention to use a non-default
GMainContext for these operations? I have memories of seeing such a
patch, but could not find it on the mailing list.

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux