From c91bb1ba1360006c568db37438779e525868cf17 Mon Sep 17 00:00:00 2001 From: Pedro Gimeno Date: Mon, 19 May 2008 17:52:00 -0300 Subject: [PATCH] Fix periodic focus bug Pedro Gimeno explains: "As you most likely already know, every X Window event comes with a timestamp. You can see it using e.g. xev. I don't know if this is Debian-specific and I'm no X Window expert, but the fact is that in my machine this timestamp, according to my /usr/include/X11/X.h, is a 32-bit unsigned integer and it holds the event time in milliseconds according to my tests. This timestamp appears to be in milliseconds from the start of the Unix epoch, modulo 2^32. The problem is that 32 bits are too little for this purpose: 2^32 milliseconds are exactly 49 days, 17 hours, 2 minutes and 47.296 seconds. Now, the WindowMaker code rejects any focus event whose timestamp is less than the last one seen, using code similar to this: if (timestamp < LastTimeStamp) return; LastTimeStamp = timestamp; This is a disaster when timestamp wraps around because of the 32-bit limit. Say LastTimeStamp equals 2^32-1 and a focus event comes two milliseconds after. Its timestamp will equal 1 because of the wraparound. Obviously, 1 < 2^32-1 so the event will be rejected even if it comes from the future (relative to LastTimeStamp) and not from the past. Focus events are no longer accepted because nothing can be greater than 2^32-1 (unless you click on that exact millisecond, 49 days after). If you look in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=102314 you'll see two people confirming the periodicity and predictability of the bug. Arnaud Giersch was who noted that the period was 2^32 milliseconds (and not 2^31 as I thought it could be because of a signed int involved) and who noted the coincidence with the Unix epoch. My fix works by subtracting one timestamp from the other (an operation which is done modulo 2^32) and rejecting the focus event only if the new timestamp is within 1 minute (60000 milliseconds) in the past. Given the periodicity of the timestamp, actually after 49.7 days there will be another minute in the future during which focus events will not be accepted; again after 99.4 days, etc., but thanks to the subtraction, they will now be relative to the *last* focus change and not to the Unix epoch. The patch could be simplified to get rid of the compareTimes function. It's written like that because it comes from the previous patch (which you can see in the same bug report). So the date comparison line could end up looking like this: if (scr->flags.ignore_focus_events || LastFocusChange - timestamp < 60000) so you can get rid of the compareTimes function. However please respect the other change of 'int' to 'Time'. As for why you aren't experiencing this problem, perhaps the event timestamp does not come from the Unix epoch in your X Window system, but rather starts counting when you start it or your computer, I don't know. Or maybe your X Window server's timestamps are 64 bits wide instead of 32 bits, which I don't think because I doubt the X11 protocol specifies a 64-bit record for the transmission of the timestamp. In either case, the output of xev will probably help understanding the cause. Or maybe even it's a click-to-focus only bug and you're not using that mode. I haven't checked who calls wSetFocusTo. Note that I can reproduce it just by manipulating the system clock, which seems to be tied to the event timestamps." For more information see: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=102314 Reported-by: Joey Hess Signed-off-by: Pedro Gimeno --- src/actions.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/actions.c b/src/actions.c index 0b3acbea..1ecaaa4c 100644 --- a/src/actions.c +++ b/src/actions.c @@ -78,6 +78,15 @@ static struct { #define SHADE_STEPS shadePars[(int)wPreferences.shade_speed].steps #define SHADE_DELAY shadePars[(int)wPreferences.shade_speed].delay +static int +compareTimes(Time t1, Time t2) +{ + Time diff; + if (t1 == t2) + return 0; + diff = t1 - t2; + return (diff < 60000) ? 1 : -1; +} /* *---------------------------------------------------------------------- @@ -99,11 +108,11 @@ wSetFocusTo(WScreen *scr, WWindow *wwin) WWindow *old_focused; WWindow *focused=scr->focused_window; - int timestamp=LastTimestamp; + Time timestamp=LastTimestamp; WApplication *oapp=NULL, *napp=NULL; int wasfocused; - if (scr->flags.ignore_focus_events || LastFocusChange > timestamp) + if (scr->flags.ignore_focus_events || compareTimes(LastFocusChange, timestamp) > 0) return; if (!old_scr) -- 2.11.4.GIT