RE: [reSIProcate] A bug in Timer.cxx
I don't think the timeGetTime function in windows is going to do exactly
what we want. It returns a system time in milliseconds and wraps around
every 49 days. The problem with this occurs in the wrap around cases.
Basically the same problem that you identified with the current code except
every 49 days instead of every 24 hours.
After reading the documentation on the SYSTEMTIME structure, it looks like
there is a way to get a 64 bit value directly from the result of the call to
GetSystemTime. This looks like the right thing to do.
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
> [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx]On Behalf
> Of yuhuicai
> Sent: Friday, July 23, 2004 11:03 AM
> To: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: [reSIProcate] A bug in Timer.cxx
>
>
> I found a bug in Timer::getSystemTime() under windows system:
>
> UInt64
> Timer::getSystemTime()
> {
> assert( sizeof(UInt64) == 64/8 );
> UInt64 time=0;
> #if defined(WIN32)
> SYSTEMTIME t;
> GetSystemTime( &t );
> time = (t.wHour*60+t.wMinute)*60+t.wSecond;
> time = time*1000000;
> time += t.wMilliseconds*1000;
> #else
> struct timeval now;
> gettimeofday( &now , NULL );
> //assert( now );
> time = now.tv_sec;
> time = time*1000000;
> time += now.tv_usec;
> #endif
> return time;
> }
>
>
> This function uses t.wHour, t.wMinute, t.wSecond and
> t.wMilliseconds to produce a 64bit value. Assume at 23:59:40, the
> processServerInvite get a 200 msg and produce a TimerStaleServer timer:
>
> mController.mTimers.add(Timer::TimerStaleServer, mId, Timer::TS );
>
> this timer will be put into a multiset named TimerQueue::mTimers.
> The value returned by getSystemTime()/1000 is 82909000. 32000
> miliseconds later, the related TimerMessage object is supposed to
> be created:
> for (std::multiset<Timer>::iterator i = mTimers.begin();
> // i != mTimers.end();)
> i != mTimers.upper_bound(now);)
> {
> if (i->mType != Timer::ApplicationTimer)
> {
> TimerMessage* t = new TimerMessage(i->mTransactionId,
> i->mType, i->mDuration);
> // Leaked !ah! BUGBUG valgrind says leaked.
> // 206884 bytes in 2270 blocks are definitely lost (...)
> // by 0x8178A20: operator new(unsigned)
> // by 0x80CDF75: resip::TimerQueue::process()
> (TimerQueue.cxx:63)
> // by 0x80F6A4B: resip::Executive::processTimer()
> (Executive.cxx:52)
>
> //DebugLog (<< Timer::toData(i->mType) << " fired (" <<
> i->mTransactionId << ") adding to fifo");
> mFifo.add(t);
> }
> else
> {
> //DebugLog(<< "ApplicationTimer " << *i->getMessage());
> // application timer -- queue the payload message
> assert(i->getMessage());
> mFifo.add(i->getMessage());
> }
>
> mTimers.erase(i++);
> }
>
> but this object will never be created until 24 hours later,
> because 23:59:40 add 32000 miliseconds is 00:00:12, the value
> returned by getSystemTime()/1000 is 12000. Since 12000 <
> 82909000, the supposed TimerMessage object won't be created until
> 24 hours later. This will let the associated transaction existed
> for about 24 hours and seems like a memory leak.
>
> To fix this bug, Timer::getSystemTime() should be changed:
>
> UInt64
> Timer::getSystemTime()
> {
> assert( sizeof(UInt64) == 64/8 );
> UInt64 time=0;
> #if defined(WIN32)
> time = timeGetTime()*1000;
>
> #else
> struct timeval now;
> gettimeofday( &now , NULL );
> //assert( now );
> time = now.tv_sec;
> time = time*1000000;
> time += now.tv_usec;
> #endif
> return time;
> }
>
> MSDN said: "The timeGetTime function retrieves the system time,
> in milliseconds. The system time is the time elapsed since
> Windows was started."
>
> So, the later time will > the former time.:)
>
>
>
>
>
>
>
>