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

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