locks in TimerThread::run, colorful all thread backtrace info if corefile generated, testcase for removeTimerUser

.gitignore

@@ -15,4 +15,6 @@ test.out
 test/generated_main.cpp
 test/test
 leak.log
+leak.log.core.*
+gdb.out
 

CMakeLists.txt

@@ -30,7 +30,8 @@ COMMAND rm -rf test/cmake_install.cmake
 COMMAND rm -rf test/CMakeCache.txt
 COMMAND rm -rf test/CTestTestfile.cmake
 COMMAND rm -rf test/cov
-COMMAND rm -rf test/*core
+COMMAND rm -rf test/leak.log.core.*
+COMMAND rm -rf test/gdb.out
 COMMAND rm -rf test/*.gcno
 COMMAND rm -rf test/lcov*.info
 COMMAND rm -rf test/leak.log

include/TimerThread.hpp

@@ -44,7 +44,7 @@ public:
                       const time_t expiration,
                       const time_t periodTime = 0 );
 
-    bool removeTimerUser ( UserEntry userEntry );
+    bool removeTimerUser ( void* timerUser );
 
     // override to signal as well
     void stop();

src/TimerThread.cpp

@@ -28,6 +28,7 @@ void TimerThread::addTimerUser(TimerUser* user,
 {
   TRACE;
   ScopedLock sl( m_mutex );
+  if ( not m_isRunning ) return;
   UserEntry userEntry = { periodTime, user };
   m_users.insert( std::pair<time_t, UserEntry>( expiration, userEntry ) );
   m_condVar.signal();
@@ -35,15 +36,19 @@ void TimerThread::addTimerUser(TimerUser* user,
 
 
 
-bool TimerThread::removeTimerUser ( UserEntry userEntry )
+bool TimerThread::removeTimerUser ( void* timerUser )
 {
   TRACE;
   ScopedLock sl( m_mutex );
-  std::multimap<time_t, UserEntry>::iterator it;
+  if ( not m_isRunning ) return false;
+  std::multimap<time_t, UserEntry>::iterator it, tmp;
   bool found(false);
-  for ( it = m_users.begin(); it != m_users.end(); it++ ) {
-    if ( it->second.user == userEntry.user ) {
-      m_users.erase(it);
+  for ( it = m_users.begin(); it != m_users.end(); ) {
+    tmp = it++;
+
+    /// @todo solveth e abstract pointer problem
+    if ( (void*)(it->second.user) == (void*)timerUser ) {
+      m_users.erase(tmp);
       m_condVar.signal();
       found = true;  // one usercan be registered multiple times
     }
@@ -73,25 +78,24 @@ void* TimerThread::run( void )
 
   while( m_isRunning ) {
 
+    m_mutex.lock();
     while ( m_users.empty() and m_isRunning ) {
       m_condVar.wait();
     }
+    m_mutex.unlock();
 
-    if ( not m_isRunning) {
-      LOG( Logger::FINEST, "return empty handed");
-      return 0;
-    }
-
+    if ( not m_isRunning) return 0;
     nextExpiration = m_users.begin()->first;
 
+    m_mutex.lock();
     // timer deleted / added, get nextExpiration again
     if ( m_condVar.wait( nextExpiration ) != ETIMEDOUT ) {
       continue;
     }
+    m_mutex.unlock();
 
     // notify & remove
-    /// @todo lock here?
-    //     m_mutex.lock();
+    m_mutex.lock();
     ret = m_users.equal_range( nextExpiration );
 
     /// @todo modify key values in multimap, must be a better way
@@ -103,12 +107,11 @@ void* TimerThread::run( void )
     }
     m_users.erase( nextExpiration );
     m_users.insert( tmp.begin(), tmp.end() );
-    //     m_mutex.unlock();
+    m_mutex.unlock();
 
   }
 
   if ( not m_users.empty() ) {
-    LOG( Logger::FINEST, "return full handed");
     for ( it = m_users.begin(); it != m_users.end(); it++ ) {
       it->second.user->timerDestroyed();
     }

test/gdb_out_parser.pl

@@ -0,0 +1,35 @@
+#!/usr/bin/perl
+
+use Term::ANSIColor qw(:constants);
+$Term::ANSIColor::AUTORESET = 1;
+
+
+while ( <> ) {
+
+  if (/^(Thread \d+)/) {
+     print "\n" . BOLD BLUE . $1 . RESET . " $'"
+  }
+  elsif ( /^(#\d+)\s+(\S+)\s+in\s+(.+)( \(.*)(at.*\/)(\S+):(\d+)/ )
+  {
+      print "\n" .
+            BOLD . $1 .          # no
+            RESET . " $2 in " .  # mem in
+            GREEN . $3 .         # function
+            RESET . " $4\n$5" .  # args, path
+            BOLD CYAN . "$6" .   # filename
+            RESET . ":" .
+            YELLOW . $7 .        # line
+            RESET . "\n";
+  }
+  elsif ( /^(#\d+)\s+(\S+)\s+in\s+(.+)( \(.*)(from.*\/)(\S+)/ )
+  {
+      print "\n" .
+            BOLD . $1 .          # no
+            RESET . " $2 in " .  # mem in
+            GREEN . $3 .         # function
+            RESET . " $4\n$5" .  # args, path
+            BLUE . "$6" .        # filename
+            RESET . "\n";
+    }
+
+}

test/run_test.sh

@@ -4,18 +4,18 @@
 #   ./run_test.sh <TEST_BINARY>
 
 
-function yesno()
-{
-  while true; do
-      read -p "$* (y/n)[y]" yn
-      if [ "$yn" = '' ]; then yn="y"; fi
-      case "$yn" in
-          [Yy]* ) return 0;;
-          [Nn]* ) return 1;;
-          * ) echo "Please answer y/n.";;
-      esac
-  done
-}
+# function yesno()
+# {
+#   while true; do
+#       read -p "$* (y/n)[y]" yn
+#       if [ "$yn" = '' ]; then yn="y"; fi
+#       case "$yn" in
+#           [Yy]* ) return 0;;
+#           [Nn]* ) return 1;;
+#           * ) echo "Please answer y/n.";;
+#       esac
+#   done
+# }
 
 pre="\E[00;33m"
 fail="\E[00;31m"
@@ -76,9 +76,12 @@ if [ $retval -ne 0 ]; then
       if [ "$cores" != "" ]; then
         echo -e "${pre}Core file generated: ${post}"
         echo $cores
-        if yesno "run 'gdb $test $cores' ?"; then
-          gdb $test $cores
-        fi
+#         if yesno "run 'gdb $test $cores' ?"; then
+#           gdb $test $cores
+#         fi
+        # NOTE no need to bt full
+        gdb $test $cores -ex "set width 1000" -ex "thread apply all bt" -ex q > gdb.out
+        ./gdb_out_parser.pl gdb.out
       fi
       exit -1
 fi

test/test_Thread.hpp

@@ -73,9 +73,10 @@ private:
        */
       sleep(665);
 
-      void* retVal = malloc(sizeof(int));
-      *((int*)retVal) = 15;
-      return retVal;
+//       void* retVal = malloc(sizeof(int));
+//       *((int*)retVal) = 15;
+//       return retVal;
+      return 0;
     }
 
     static void signal_handler(int sig)
@@ -105,7 +106,7 @@ public:
     TS_ASSERT(retVal);
     if (retVal != 0 ) {
       TS_ASSERT_EQUALS ( *((int*)retVal) , 16 );
-      free(retVal);
+      free((int*)retVal);
     }
     delete m2;
   }

test/test_TimerThread.hpp

@@ -88,4 +88,27 @@ public:
     delete user;
   }
 
+  void testRemoved( void )
+  {
+    TEST_HEADER;
+    TimerThread* tt = new TimerThread();
+    tt->start();
+    sleep(1);
+
+    DummyTimerUser *user = new DummyTimerUser();
+    tt->addTimerUser( user, 10 );
+
+    sleep(2);
+    tt->removeTimerUser( user );
+
+    sleep(1);
+    tt->stop();
+    sleep(1);
+
+    TS_ASSERT_EQUALS( user->m_counter, 100 );
+
+    delete tt;
+    delete user;
+  }
+
 };

test/valgrind.supp

@@ -5,7 +5,7 @@
    fun:dl_main
    fun:_dl_sysdep_start
    fun:_dl_start
-   obj:/lib64/ld-2.13.so
+   obj:/lib64/ld-*.so
 }
 
 {
@@ -15,7 +15,7 @@
    fun:dl_main
    fun:_dl_sysdep_start
    fun:_dl_start
-   obj:/lib64/ld-2.13.so
+   obj:/lib64/ld-*.so
 }
 
 {
@@ -25,7 +25,7 @@
    fun:dl_main
    fun:_dl_sysdep_start
    fun:_dl_start
-   obj:/lib64/ld-2.13.so
+   obj:/lib64/ld-*.so
 }
 
 {
@@ -35,7 +35,7 @@
    fun:dl_main
    fun:_dl_sysdep_start
    fun:_dl_start
-   obj:/lib64/ld-2.13.so
+   obj:/lib64/ld-*.so
 }
 
 {
@@ -43,7 +43,8 @@
    Memcheck:Leak
    fun:calloc
    fun:_dl_allocate_tls
-   fun:pthread_create@@GLIBC_2.2.5
+   fun:pthread_create@@GLIBC_*
 }
 
 
+