TimerThread corrections

include/TimerThread.hpp

@@ -48,7 +48,7 @@ public:
                        const timespec expiration,
                        const timespec periodTime = timespec_ctor() );
 
-    bool removeTimerUser ( void* timerUser );
+    bool removeTimerUser( TimerUser* timerUser );
 
     // override to signal as well
     void stop();
@@ -56,9 +56,11 @@ public:
 
 private:
 
+  void notifyAndRemove( const timespec t );
+
   void* run( void );
 
-  // facory function
+  // ctor function
   inline static timespec timespec_ctor() {
     timespec tmp = { 0, 0 };
     return tmp;

src/TimerThread.cpp

@@ -4,12 +4,11 @@
 #include "ScopedLock.hpp"
 
 #include <errno.h> // ETIMEDOUT
-#include <pthread.h> // pthread_attr_t, sched_param
 
 
 
 TimerThread::TimerThread()
-  : Thread(true)
+  : Thread()
   , m_mutex()
   , m_condVar(m_mutex)
   , m_users()
@@ -29,19 +28,11 @@ void TimerThread::addTimerUser(TimerUser* user,
                                const time_t periodTime)
 {
   TRACE;
-  ScopedLock sl( m_mutex );
-  if ( not m_isRunning ) return;
-  timespec expirationTS = { expiration, 0 };
-  timespec periodTimeTS = { periodTime, 0 };
-
-  UserEntry userEntry = { periodTimeTS, user };
-  m_users.insert( std::pair<timespec, UserEntry>( expirationTS, userEntry ) );
 
-  std::string s;
+  timespec periodTimeTS = { periodTime, 0 };
-  s = std::string("adding user: ") + stringify(user);
+  timespec expirationTS = { expiration, 0 };
-  LOG(Logger::FINEST, s.c_str());
 
-  m_condVar.signal();
+  addTimerUser( user, expirationTS, periodTimeTS );
 }
 
 
@@ -52,35 +43,30 @@ void TimerThread::addTimerUser( TimerUser* user,
   TRACE;
   ScopedLock sl( m_mutex );
   if ( not m_isRunning ) return;
-  UserEntry userEntry = { periodTime, user };
-  m_users.insert( std::pair<timespec, UserEntry>( expiration, userEntry ) );
 
-  std::string s;
+  timespec ts;
-  s = std::string("adding user: ") + stringify(user);
+  clock_gettime( CLOCK_REALTIME, &ts );
-  LOG(Logger::FINEST, s.c_str());
+  ts = timespecAdd ( ts, expiration );
+
+  UserEntry userEntry = { periodTime, user };
+  m_users.insert( std::pair<timespec, UserEntry>( ts, userEntry ) );
 
   m_condVar.signal();
 }
 
 
-bool TimerThread::removeTimerUser ( void* timerUser )
+bool TimerThread::removeTimerUser( TimerUser* timerUser )
 {
   TRACE;
   ScopedLock sl( m_mutex );
   if ( not m_isRunning ) return false;
+
   std::multimap<timespec, UserEntry>::iterator it, tmp;
   bool found(false);
   for ( it = m_users.begin(); it != m_users.end(); ) {
 
-    std::string s;
-    s = stringify(it->second.user) + " = ? = " + stringify(timerUser);
-    LOG(Logger::FINEST, "Checking user to delete...does it match? ");
-    LOG(Logger::FINEST, s.c_str());
-
-    /// @todo solve the abstract pointer problem
     if ( (it->second.user) == timerUser ) {
       tmp = it++;
-      LOG(Logger::FINEST, "Removing a user.");
       m_users.erase(tmp);
       m_condVar.signal();
       found = true;  // one user can be registered multiple times
@@ -91,7 +77,7 @@ bool TimerThread::removeTimerUser ( void* timerUser )
   return found;
 }
 
-// override to signal as well
+
 void TimerThread::stop()
 {
   TRACE;
@@ -101,62 +87,78 @@ void TimerThread::stop()
 }
 
 
-void* TimerThread::run( void )
+void TimerThread::notifyAndRemove( const timespec t )
 {
   TRACE;
-  timespec nextExpiration;
+  ScopedLock sl( m_mutex );
+  if ( not m_isRunning ) return;
 
+  timespec ts;
   std::multimap<timespec, UserEntry, timespec_cmp> tmp;
   std::multimap<timespec, UserEntry>::iterator it;
   std::pair<std::multimap<timespec, UserEntry>::iterator,
             std::multimap<timespec, UserEntry>::iterator> ret;
 
+  ret = m_users.equal_range( t );
+
+  /// @todo modify key values in multimap, must be a better way
+  tmp.clear();
+  for ( it = ret.first; it != ret.second; it++ ) {
+    it->second.user->timerExpired();
+    if ( it->second.periodTime.tv_sec != 0 or it->second.periodTime.tv_nsec != 0) {
+
+      clock_gettime( CLOCK_REALTIME, &ts );
+      ts = timespecAdd( ts, it->second.periodTime );
+
+      tmp.insert(std::pair<timespec, UserEntry>( ts, it->second ) );
+      m_condVar.signal();
+    }
+  }
+  m_users.erase( t );
+  m_users.insert( tmp.begin(), tmp.end() );
+}
+
+
+void* TimerThread::run( void )
+{
+  TRACE;
+  timespec nextExpiration;
+  timespec ts, wait;
+
   while( m_isRunning ) {
 
     m_mutex.lock();
     while ( m_users.empty() and m_isRunning ) {
-      LOG(Logger::FINEST, "Waiting for a user, since the map is empty.");
       m_condVar.wait();
     }
     m_mutex.unlock();
 
     if ( not m_isRunning) return 0;
+
     nextExpiration = m_users.begin()->first;
+    clock_gettime( CLOCK_REALTIME, &ts );
 
-    m_mutex.lock();
+    wait = timespecSubstract ( nextExpiration, ts );
-    // timer deleted / added, get nextExpiration again
+    if ( wait.tv_sec == 0 and wait.tv_nsec == 0 ) {
-    if ( m_condVar.wait( nextExpiration.tv_sec,
+      notifyAndRemove ( nextExpiration );
-                         nextExpiration.tv_nsec ) != ETIMEDOUT ) {
-      LOG(Logger::FINEST, "Abort sleep: user has been added/removed.");
-      m_mutex.unlock();
       continue;
     }
-    m_mutex.unlock();
-
-    // notify & remove
     m_mutex.lock();
-    ret = m_users.equal_range( nextExpiration );
+
-
+    if ( m_condVar.wait( wait.tv_sec,
-    /// @todo modify key values in multimap, must be a better way
+                         wait.tv_nsec ) != ETIMEDOUT ) {
-    tmp.clear();
+      m_mutex.unlock();
-    for ( it = ret.first; it != ret.second; it++ ) {
+      continue;
-      LOG(Logger::FINEST, "Notifying one user.");
-      it->second.user->timerExpired();
-      if ( it->second.periodTime.tv_sec != 0 or it->second.periodTime.tv_nsec != 0) {
-        LOG(Logger::FINEST, "Periodic, re-adding.");
-        tmp.insert(std::pair<timespec, UserEntry>(
-                  it->second.periodTime, it->second ) );
-      }
     }
-    m_users.erase( nextExpiration );
-    m_users.insert( tmp.begin(), tmp.end() );
     m_mutex.unlock();
 
+    notifyAndRemove( nextExpiration );
   }
 
+  // end ...
   if ( not m_users.empty() ) {
+    std::multimap<timespec, UserEntry>::iterator it;
     for ( it = m_users.begin(); it != m_users.end(); it++ ) {
-      LOG(Logger::FINEST, "Calling timerDestroyed on one user.");
       it->second.user->timerDestroyed();
     }
   }