XDM and 23 Jun 10:27:13

[Rob Currey]

Personally, I luv this bug ...

A customer of ours reported that on EXACTLY 23 Jun 10:27:13 their XDM logged 
in sessions would reset back to the XDM login screen (note: I don't believe 
the X server actually does a Reset, but just blammo back to XDM login).

Our logs showed a line as ...
[23 Jun 10:27:13]: XDM: too many keepalive retransmissions, declaring
session dead X connection to lena:1.0 broken (explicit kill or server
shutdown).

I should also note, that this is from an X server based upon XFree 4.x and 
not the latest X.org tree. In any event, I believe this bug is still 
present.

In researching the problem I came across this ...
 http://lists.debian.org/debian-x/2002/02/msg00033.html
which then made me realize this wasn't a problem with our DDX, but something 
else ... (note our case was upon HPUX, but ...)

So, I think I've nailed it down to thus ...
First, if DDXTIME is not defined (in XFree86 ... I don't see this even 
possible in X.org), then the value returned is X_GETTIMEOFDAY*1000 (which 
would be seconds since epoch times 1000). Again the latest X.org tree might 
be implementing X_GETTIMEOFDAY differently now?
The calculations using this value are done via CARD32 (and a subtraction of 
a default 3 seconds), resulting in the calculation being done with an 
overflowed value and a value that wasn't ... a <0 value results and whammo 
XDM thinks it's timedout.
I'd appreciate if someone could very my math and the functions being done in 
xserver/os/xdmcp.c at that date ... 
http://www.perlservices.net/en/programs/epoch_converter/epoch_converter.html 
may be useful :)

In any event, It looks to me that xdmcp.c should NOT be doing the time 
calculation in millis anyway! The "dormancy" is in seconds, so keep the 
values in seconds (not go to millis, have rollover issues due to limited 
data types, etc)

So, I propose a fix for this is to have xdmcp.c NOT use GetTimeInMillis() 
but instead do something along the lines of ...

bash-2.04$ diff xdmcp.c.new xdmcp.c
264,272d263
< CARD32
< GetTimeInSeconds(void)
< {
<     struct timeval  tp;
<
<     X_GETTIMEOFDAY(&tp);
<     return tp.tv_sec;
< }
<
718c709
<     CARD32 secsToGo;
---
>     CARD32 millisToGo;
729,732c720,723
<     secsToGo = timeOutTime - GetTimeInSeconds();
<     if ((int) secsToGo < 0)
<       secsToGo = 0;
<     AdjustWaitForDelay (wt, secsToGo*1000);
---
>     millisToGo = timeOutTime - GetTimeInMillis();
>     if ((int) millisToGo < 0)
>       millisToGo = 0;
>     AdjustWaitForDelay (wt, millisToGo);
776c767
<           timeOutTime = GetTimeInSeconds() +  keepaliveDormancy;
---
>           timeOutTime = GetTimeInMillis() +  keepaliveDormancy * 1000;
778c769
<     else if (timeOutTime && (int) (GetTimeInSeconds() - timeOutTime) >= 0)
---
>     else if (timeOutTime && (int) (GetTimeInMillis() - timeOutTime) >= 0)
914c905
<     timeOutTime = GetTimeInSeconds() + rtx;
---
>     timeOutTime = GetTimeInMillis() + rtx * 1000;
1505,1506c1496,1497
<           if ((GetTimeInSeconds() - 
(lastDeviceEventTime.milliseconds/1000)) >
<               keepaliveDormancy )
---
>           if ((GetTimeInMillis() - lastDeviceEventTime.milliseconds) >
>               keepaliveDormancy * 1000)
1512c1503
<           timeOutTime = GetTimeInSeconds() + keepaliveDormancy;
---
>           timeOutTime = GetTimeInMillis() + keepaliveDormancy * 1000;

('m sorry I don't have a proper patch/diff against the latest version in 
git, I'm still working to get there ...)

Rob