(timeout in ms vs. XSyncValueSubtract) Frozen client, found cause, need advise for fix

[Daniel Martin]

On 21 February 2017 at 17:48, walter harms <wharms at bfs.de> wrote:
> Am 20.02.2017 10:55, schrieb Daniel Martin:
>> What happens is:
>> - ServertimeBlockHandler() forwards a _big_ unsigned timeout to
>> AdjustWaitForDelay()
>> - AdjustWaitForDelay() takes this _big_ unsigned as int and it becomes
>> a negative value
>> - the negative value ends up as timeout in ospoll_wait() as parameter
>> to epoll_wait()
>>> epoll_wait() blocks due to the negative timeout
>>
>> XSyncValueSubtract() doesn't handle unsigned wraps. That's why we get
>> a big unsigned from ServertimeBlockHandler(). Just imagine two
>> XSyncValues:
>>     a = {.hi=1, .lo=1} and b = {.hi=0, lo=.2}.
>> The result of XSyncValueSubtract(a - b) is
>>     presult = {.hi=0, .lo=4294967295},
>> where I would expect
>>    presult = {.hi=0, .lo=9}.
>>
>> If .lo=4294967295 is wrong and .lo=9 is what we want, could anyone
>> provide a fix, please? I'm to dumb for the (binary?) arithmetic atm.
>> (And if it is wrong, did I just uncovered an ancient bug?)
>>
>
> XSyncValueSubtract is doing as expected,
> XSyncValue is the simulation of 64bitvalues on 32bit.
> see this in hex:
>  100000001
> -000000002
>  0FFFFFFFF = 4294967295 in .lo

If XSyncValueSubtract is correct as is, then I would say, that it is
not what we want in ServertimeBlockHandler().
At that point we handle XSyncValues like a struct timeval, though .lo
not being in usec, but ms. So, every .lo < 0 or > 999 is wrong. We
would need something like:

void  XSyncValueSubtractMS(XSyncValue *presult, XSyncValue a,
XSyncValue b, Bool *poverflow) {
    Bool signa = XSyncValueIsNegative(a);
    Bool signb = XSyncValueIsNegative(b);

    XSyncValue tmp = {.hi=b.hi, .lo=b.lo};

    if (a.lo < tmp.lo) {
        int nhi = (tmp.lo - a.lo) / 1000 + 1;
        tmp.lo -= 1000 * nhi;
        tmp.hi += nhi;
    }
    if (a.lo - tmp.lo > 1000) {
        int nhi = (a.lo - tmp.lo) / 1000;
        tmp.lo += 1000 * nhi;
        tmp.hi -= nhi;
    }

    presult->hi = a.hi - tmp.hi;
    presult->lo = a.lo - tmp.lo;

    *poverflow = ((signa == signb) && !(signa ==
XSyncValueIsNegative(*presult)));
} /* adopted from
https://www.gnu.org/software/libc/manual/html_node/Elapsed-Time.html
*/

Or convert our XSyncValues to timevals and use timersub().

Cheers,
    Daniel