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 <[email protected]> > > > > 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;
}
signature.asc
Description: PGP signature
_______________________________________________ Libosinfo mailing list [email protected] https://www.redhat.com/mailman/listinfo/libosinfo
