From 8e1dbfa5690978b87ad5a60f4efb2f28e28d913a Mon Sep 17 00:00:00 2001
From: Denes Matetelki <denes.matetelki@gmail.com>
Date: Sat, 2 Apr 2011 22:59:50 +0200
Subject: [PATCH] bug fixed ad removing user in TimerThread. TimerThread has
 some logs, till complete testing

---
 src/TimerThread.cpp       | 31 +++++++++++++++++++++++++++----
 test/test_TimerThread.hpp | 23 ++++++++++-------------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/src/TimerThread.cpp b/src/TimerThread.cpp
index 7e534d7..82cd71f 100644
--- a/src/TimerThread.cpp
+++ b/src/TimerThread.cpp
@@ -36,6 +36,11 @@ void TimerThread::addTimerUser(TimerUser* user,
 
   UserEntry userEntry = { periodTimeTS, user };
   m_users.insert( std::pair<timespec, UserEntry>( expirationTS, userEntry ) );
+
+  std::string s;
+  s = std::string("adding user: ") + stringify(user);
+  LOG(Logger::FINEST, s.c_str());
+
   m_condVar.signal();
 }
 
@@ -49,6 +54,11 @@ void TimerThread::addTimerUser( TimerUser* user,
   if ( not m_isRunning ) return;
   UserEntry userEntry = { periodTime, user };
   m_users.insert( std::pair<timespec, UserEntry>( expiration, userEntry ) );
+
+  std::string s;
+  s = std::string("adding user: ") + stringify(user);
+  LOG(Logger::FINEST, s.c_str());
+
   m_condVar.signal();
 }
 
@@ -61,10 +71,16 @@ bool TimerThread::removeTimerUser ( void* timerUser )
   std::multimap<timespec, UserEntry>::iterator it, tmp;
   bool found(false);
   for ( it = m_users.begin(); it != m_users.end(); ) {
-    tmp = it++;
+
+    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 ( (void*)(it->second.user) == (void*)timerUser ) {
+    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
@@ -79,7 +95,7 @@ void TimerThread::stop()
   TRACE;
   ScopedLock sl( m_mutex );
   m_isRunning = false;
-  m_condVar.signal();
+  m_condVar.broadcast();
 }
 
 
@@ -97,6 +113,7 @@ void* TimerThread::run( void )
 
     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();
@@ -108,6 +125,8 @@ void* TimerThread::run( void )
     // timer deleted / added, get nextExpiration again
     if ( m_condVar.wait( nextExpiration.tv_sec,
                          nextExpiration.tv_nsec ) != ETIMEDOUT ) {
+      LOG(Logger::FINEST, "Abort sleep: user has been added/removed.");
+      m_mutex.unlock();
       continue;
     }
     m_mutex.unlock();
@@ -119,10 +138,13 @@ void* TimerThread::run( void )
     /// @todo modify key values in multimap, must be a better way
     tmp.clear();
     for ( it = ret.first; it != ret.second; it++ ) {
+      LOG(Logger::FINEST, "Notifying one user.");
       it->second.user->timerExpired();
-      if ( it->second.periodTime.tv_sec != 0 or it->second.periodTime.tv_nsec != 0)
+      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() );
@@ -132,6 +154,7 @@ void* TimerThread::run( void )
 
   if ( not m_users.empty() ) {
     for ( it = m_users.begin(); it != m_users.end(); it++ ) {
+      LOG(Logger::FINEST, "Calling timerDestroyed on one user.");
       it->second.user->timerDestroyed();
     }
   }
diff --git a/test/test_TimerThread.hpp b/test/test_TimerThread.hpp
index c64b13d..59932ef 100644
--- a/test/test_TimerThread.hpp
+++ b/test/test_TimerThread.hpp
@@ -154,20 +154,22 @@ public:
     tt->addTimerUser( user, 10 );
 
     sleep(2);
+    TS_ASSERT_EQUALS( tt->m_users.size(), 1);
+
+
     tt->removeTimerUser( user );
 
     sleep(1);
+    TS_ASSERT_EQUALS( tt->m_users.size(), 0);
+
     tt->stop();
     sleep(1);
 
-    TS_ASSERT_EQUALS( user->m_counter, 100 );
-
     delete tt;
     delete user;
   }
 
-  /// @bug this case does not work, investigate!
-  void bugtestRemovedMultiple( void )
+  void testRemovedMultiple( void )
   {
     TEST_HEADER;
     TimerThread* tt = new TimerThread();
@@ -176,17 +178,15 @@ public:
 
     DummyTimerUser *user = new DummyTimerUser();
     tt->addTimerUser( user, 10 );
-    DummyTimerUser *user2 = new DummyTimerUser();
-    tt->addTimerUser( user2, 12 );
-    DummyTimerUser *user3 = new DummyTimerUser();
-    tt->addTimerUser( user3, 13 );
-    DummyTimerUser *user4 = new DummyTimerUser();
-    tt->addTimerUser( user4, 14 );
+    tt->addTimerUser( user, 12 );
+    tt->addTimerUser( user, 13 );
+    tt->addTimerUser( user, 14 );
 
     TS_ASSERT_EQUALS( tt->m_users.size(), 4);
 
     sleep(2);
     tt->removeTimerUser( user );
+    sleep(2);
 
     TS_ASSERT_EQUALS( tt->m_users.size(), 0);
 
@@ -197,9 +197,6 @@ public:
 
     delete tt;
     delete user;
-    delete user2;
-    delete user3;
-    delete user4;
   }
 
 };