RE: RE: [reSIProcate] A bug in Timer.cxx
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.:)
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
>