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