GNOME Bugzilla – Bug 448943
g_timeout_add_seconds() problems
Last modified: 2008-04-08 09:40:26 UTC
When using g_timeout_add_seconds(), sometimes the timeout appears to never trigger until there is some sort of other event (e.g., when I press the ctrl or alt key on my keyboard). When I changed my test program to use g_timeout_add (seconds * 1000, ...) I could not duplicate this behavior. This is happening in glib trunk.
Created attachment 90243 [details] Test program Sorry, I'm recycling my test program from bug 56070 here and I realize it's not a "minimal" test of the bug. If that's unacceptable let me know and I'll cut it down. You may have to run this multiple times to reproduce the bug, because it seems to trigger inconsistently for me.
I can reproduce this too with my Farsight2 unit test, the timeout passed g_main_context_poll() is MAXINT.
Created attachment 102297 [details] testcase I failed to reproduce this with a simpler test case. Can you ?
Sorry, I suddenly can't reproduce it using either your simple test program or the test program from bug #56070 which is where I first noticed this.
I can reproduce with matthias's testcase after a few tries and glib 2.14.6. The timeout for the poll() is MAXINT. And more interesting is that the tv_usec of the source's expiration is -71374 (<0). Something is wrong there. So in g_timeout_prepare, we have sec>0 and msec<-1000 .. and then it does (guint)msec.. which becomes a huge value so it chooses G_MAXINT... One fix is in g_timeout_prepare(), to replace "if (msec < 0)", by "while (msec < 0). Also, in g_timeout_set_expiration(), if the remainder is larger than the perturb, that can cause a negative tv_usec value..
I'm confirming this bug, and we should fix this asap... It makes the whole api useless
Do you have a proposed patch ?
Created attachment 107805 [details] [review] Ensure the perturb value is always positive g_timeout_prepare is fine as long as the initially calculated msec is: -1000 < msec < 1000, which is true as long as the tv_usecs are: 0 < tv_usec < 1000 * 1000.. Now if we look in g_timeout_set_expiration, then this is the case as long as timer_perturb >= 0. Just do the code by hand for timber_perturb = -10, gran = 50 and expiration.tv_usec = 0.. Unfortunately (gint)g_str_hash isn't guaranteed to be >= 0, hence this trivial fix..
2008-04-03 Matthias Clasen <mclasen@redhat.com> Bug 448943 – g_timeout_add_seconds() problems * glib/gmain.c (g_timeout_set_expiration): Prevent expiration time going negative. Reported by Cody Russell, analyzed by Olivier Crete, patch by Sjoerd Simons.
Unfortunately the commited patch is slightly different then my patch and wrong... Committed is: - timer_perturb = g_str_hash (session_bus_address); + timer_perturb = ABS (g_str_hash (session_bus_address)); while my patch did: - timer_perturb = g_str_hash (session_bus_address); + timer_perturb = ABS ((gint) g_str_hash (session_bus_address)); Note that the cast to gint before using was removed. As g_str_hash returns a guint the commited patch doesn't actually have any effect at all :(.
I swear I looked and removed the cast because the function already returned a gint...now that I look again, you are of course right, it returns guint. Sorry about that, correcting it now.
Note that you're calling g_str_hash() once more than needed since ABS() evaluates the argument twice. I have verified that adding G_GNUC_PURE to g_str_hash() fixes the problem. I suggest to go through the GLib API and add optimization attributes as appropriate.