< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>