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

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.:)
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
>