Re: RE: RE: [reSIProcate] A bug in Timer.cxx
The FILETIME structure is a 64-bit value representing the number of
100-nanosecond intervals since January 1, 1601.
So, the value returned by li.QuadPart should be divided by 10:
UInt64
Timer::getSystemTime()
{
assert( sizeof(UInt64) == 64/8 );
UInt64 time=0;
#if defined(WIN32)
SYSTEMTIME t;
GetSystemTime( &t );
FILETIME ft;
SystemTimeToFileTime( &t, &ft);
ULARGE_INTEGER li;
li.LowPart = ft.dwLowDateTime;
li.HighPart = ft.dwHighDateTime;
time = li.QuadPart/10;
#else
struct timeval now;
gettimeofday( &now , NULL );
//assert( now );
time = now.tv_sec;
time = time*1000000;
time += now.tv_usec;
#endif
return time;
}
> I checked in a fix to the problem.
>
> > -----Original Message-----
> > From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
> > [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx]On Behalf
> > Of yuhuicai
> > Sent: Friday, July 23, 2004 11:45 AM
> > To: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: RE: [reSIProcate] A bug in Timer.cxx
> >
> >
> > You are right, "The return value wraps around to 0 every 2^32
> > milliseconds, which is about 49.71 days", and the UINT64 is big
> > enough to hold this value:
> > 18446744073709551615 (2^64)
> > 959220000000000000 (00:00:00, Jan, 2000)
> > But is seems not easy to convert a SYSTEMTIME value to UINT64.
> > Some month has 31 days, others have 30, 29, or 28 days.
> >
> >
> > > 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.:)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> >
>
>
>