Re: [reSIProcate] Timers: why system time?
Another possible implementation. It avoids having to know when the timer
wraps so the logic stays consistent. Restriction is that no timers can be >
49 days in duration and that getTimeMs must be called at least once in 49
days. Looks like the max "getTimeTillNextProcessMs" returned for some
objects is INT_MAX, which is small enough to avoid a wrap problem.
Comments?
Thanks,
-justin
UInt64
Timer::getTimeMs(void)
{
UInt64 current = mTimeVal;
ULARGE_INTEGER timeVal;
timeVal.QuadPart = current;
DWORD tickNow =::GetTickCount();
if (tickNow != timeVal.LowPart)
{
DWORD diff = tickNow - timeVal.LowPart;
timeVal.QuadPart += diff;
if (diff > 60000)
{
//_InterlockedCompareExchange64((volatile LONGLONG
*)&mTimeVal,timeVal.QuadPart,current);
{
Lock lock(mWrapCounterMutex);
if (current == mTimeVal)
{
mTimeVal = timeVal.QuadPart;
}
}
}
else
{
return timeVal.QuadPart;
}
}
return mTimeVal;
}
-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Byron Campen
Sent: Friday, October 31, 2008 5:01 PM
To: Adam Roach
Cc: 'resiprocate-devel'
Subject: Re: [reSIProcate] Timers: why system time?
Sounds like a good idea to me.
Best regards,
Byron Campen
> Byron Campen wrote:
>> So, maybe on win32 we need to have something call getTimeMs()
>> every minute? A heartbeat timer of some sort?
>
> You could be even more clever about it than that. Inserting a timer
> immediately before and after the rollover should get you there. That
> way, you have two timers every 49 days, instead of one timer every
> minute. :)
>
> UInt64 timer1Time = 0x00000000FFFFFFF0ui64 | (Timer::getTimeMs() &
> 0xFFFFFFFF00000000ui64);
> UInt64 timer2Time = timer1 + 33;
>
> Then, all you need is a new Timer constructor that takes an absolute
> time instead of a relative one.
>
> /a
>
>>
>> Best regards,
>> Byron Campen
>>
>>> To qualify things further, though: in order for this bug to be
>>> triggered, you would need to have no calls to "Timer::getTimeMs()"
>>> for over two minutes (131.07 seconds, to be more precise) as the
>>> 49.7-day boundary rolls around. In a normally operating SIP stack,
>>> the chances of this happening seem vanishingly small.
>>>
>>> So, it's a corner case on top of a corner case. And it only happens
>>> on WIN32 systems.
>>>
>>> /a
>>>
>>> Byron Campen wrote:
>>>> Ok, so this is in the "much more serious" category. I would not
>>>> be comfortable leaving this be.
>>>>
>>>> Best regards,
>>>> Byron Campen
>>>>
>>>>> Byron Campen wrote:
>>>>>> Let's say one of these timers is at the top of the heap, and
>>>>>> whatever error you're thinking about happens. Are you saying that
>>>>>> not only will that timer be 49 days late, but everything else in
>>>>>> the heap will be too?
>>>>>
>>>>> Not new timers. But, if we miss the rollover, then everything in
>>>>> the heap at that time will instantaneously appear to be 49.7 days
>>>>> later than their original expiration time. New timers will be
>>>>> inserted in the heap just fine (ahead of the "lost" timers), and
>>>>> expire correctly.
>>>>>
>>>>> The behavior would be similar to using our current stack and
>>>>> setting the system time to be 49 days in the past while timers are
>>>>> pending.
>>>>>
>>>>> /a
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Byron Campen
>>>>>>
>>>>>>> Technically, they won't be "lost" as much as "over 49 days
>>>>>>> late." From a SIP protocol perspective, 49 days is long enough
>>>>>>> to consider the timer lost for all practical purposes.
>>>>>>>
>>>>>>> /a
>>>>>>>
>>>>>>> Byron Campen wrote:
>>>>>>>> I can see how we would miss a roll-over with low timer
>>>>>>>> activity, but I am not sure what you mean by "lose timers".
>>>>>>>> If we have lost a timer from a timer heap, that implies the
>>>>>>>> heap is now broken, because that is the only way to "lose"
>>>>>>>> anything. So this is either much more serious, or much less
>>>>>>>> serious than you have stated.
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Byron Campen
>>>>>>>>
>>>>>>>>> Scott (and any other Windows developers on the list):
>>>>>>>>>
>>>>>>>>> The Windows 49.7-day rollover fix in this patch uses
>>>>>>>>> heuristics to handle the rollover in an efficient manner --
>>>>>>>>> you can actually lose timers if you have relatively low timer
>>>>>>>>> activity and a multi-minute timer running when the tick count
>>>>>>>>> rolls over. The chances of this happening are, admittedly,
>>>>>>>>> vanishingly small (especially in normal SIP usage). In any
>>>>>>>>> case, you might want to give that code a quick review to
>>>>>>>>> ensure that you're comfortable with it.
>>>>>>>>>
>>>>>>>>> Also, I think it's safe to say that it's difficult to test the
>>>>>>>>> correctness of the code, as step one of any such test plan
>>>>>>>>> necessarily involves something like: "boot a Windows machine
>>>>>>>>> and wait 49.6 days." Make sure you're comfortable with that
>>>>>>>>> fact as well.
>>>>>>>>>
>>>>>>>>> /a
>>>>>>>>>
>>>>>>>>> Alexander wrote:
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> Here is a patch to use monotonic clock.
>>>>>>>>>> It is 3 modified files from reSIProcate 1.4.
>>>>>>>>>> Attached files have Windows (CLRF) eof-style.
>>>>>>>>>>
>>>>>>>>>> Windows
>>>>>>>>>> =======
>>>>>>>>>>
>>>>>>>>>> GetTickCount() with 16 ms inaccuracy used.
>>>>>>>>>> It is very simple to change it to timeGetTime() etc with 1 ms
>>>>>>>>>> accuracy, but it will require init/fini calls on application
>>>>>>>>>> level and linking with Winmm.lib Tested on Windows XP SP2,
>>>>>>>>>> compiled with VS7.1
>>>>>>>>>>
>>>>>>>>>> Not tested on Windows CE
>>>>>>>>>>
>>>>>>>>>> Linux
>>>>>>>>>> =====
>>>>>>>>>>
>>>>>>>>>> If monotonic clock is not available (compile time and
>>>>>>>>>> runtime) it will
>>>>>>>>>> fall back to gettimeofday()
>>>>>>>>>> I am not familiar with reSIProcate build system so I use
>>>>>>>>>> workaround to avoid linking with librt:
>>>>>>>>>> syscall( __NR_clock_gettime, ... ) instead of
>>>>>>>>>> clock_gettime(...)
>>>>>>>>>>
>>>>>>>>>> IMHO it may not work on other POSIX compliant platforms.
>>>>>>>>>>
>>>>>>>>>> Pass only some basic tests on my Linux PC (2.6.18 kernel)
>>>>>>>>>>
>>>>>>>>>> Any feedbacks appreciated.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Alexander Altshuler
>>>>>>>>>> Xeepe team
>>>>>>>>>> http://xeepe.com
>>>>>>>>>> -------------------------------------------------------------
>>>>>>>>>> -----------
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> resiprocate-devel mailing list
>>>>>>>>>> resiprocate-devel@xxxxxxxxxxxxxxx
>>>>>>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-dev
>>>>>>>>>> el
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> resiprocate-devel mailing list
>>>>>>>>> resiprocate-devel@xxxxxxxxxxxxxxx
>>>>>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-
>>>>>>>>> devel
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>