Wncxione 662 : Timer loop stuck

.github/workflows/coverity_full_scan.yml

@@ -0,0 +1,22 @@
+name: Coverity Full Analysis Scan
+
+on:
+  push:
+    branches: [ develop, main ]
+    paths: ['**/*.c', '**/*.cpp', '**/*.cc', '**/*.cxx', '**/*.h', '**/*.hpp']
+
+jobs:
+  call_coverity_full_scan:
+    uses: rdk-e/build_tools_workflows/.github/workflows/coverity_component_full_scan.yml@main
+    with:
+      branchName: ${{ github.ref_name }}
+      # Specify your actual build command here
+      buildCommand: make all
+      # Specify any packages required for building your components. 
+      # Enable the customSetup and add the package names below.
+      #customSetup: |
+      #  apt-get update -y
+      #  apt-get install -y vim
+    secrets: 
+      RDKE_ARTIFACTORY_USER_APIKEY: ${{ secrets.RDKE_ARTIFACTORY_USER_APIKEY }}
+      RDKE_COVERITY_APIKEY: ${{ secrets.RDKE_COVERITY_APIKEY }}

.github/workflows/coverity_incremental_scan.yml

@@ -0,0 +1,31 @@
+name: Coverity Incremental Analysis Scan
+
+on:
+  pull_request:
+    branches: [ develop ]
+    paths: ['**/*.c', '**/*.cpp', '**/*.cc', '**/*.cxx', '**/*.h', '**/*.hpp']
+
+  workflow_dispatch:
+    inputs:
+      pullRequestNumber:
+        description: 'Coverity Run: Pull Request Number'
+        required: true
+        type: string
+
+jobs:
+  call_coverity_incremental_scan:
+    uses: rdk-e/build_tools_workflows/.github/workflows/coverity_component_incremental_scan.yml@main
+    with:
+      pullRequestNumber: ${{ github.event.inputs.pullRequestNumber || github.event.pull_request.number }}
+      branchName: ${{ github.event.pull_request.base.ref }}
+      # Specify your actual build command here
+      buildCommand: make all
+      # Specify any packages required for building your components. 
+      # Enable the customSetup and add the package names below.
+      #customSetup: |
+      #  apt-get update -y
+      #  apt-get install -y vim
+    secrets: 
+      RDKE_ARTIFACTORY_USER_APIKEY: ${{ secrets.RDKE_ARTIFACTORY_USER_APIKEY }}
+      RDKE_COVERITY_APIKEY: ${{ secrets.RDKE_COVERITY_APIKEY }}
+      RDKE_GITHUB_TOKEN: ${{ secrets.RDKE_GITHUB_TOKEN }}

Makefile

@@ -22,10 +22,15 @@ export BUILD_DIR = $(PWD)/build
 # Source sub directories, order is important.
 SUBDIRS = src rdkperf test service
 
+.PHONY: all clean quick-exit-test
+
 all:
 	@for i in $(SUBDIRS); do \
 	echo "make all in $$i..."; \
 	(cd $$i; $(MAKE) $(MFLAGS)); done
+
+quick-exit-test: all
+	$(MAKE) -C test quick-exit-test
 
 clean:
 	@for i in $(SUBDIRS); do \

rdkperf/rdk_perf.cpp

@@ -39,8 +39,6 @@
 
 #include <unistd.h>
 
-#define REPORTING_INITIAL_COUNT 5000
-#define REPORTING_INTERVAL_COUNT 20000
 #define TIMER_INTERVAL_SECONDS 10
 #define MAX_DELAY 600
 
@@ -54,18 +52,10 @@ static void __attribute__((destructor)) PerfModuleTerminate();
 
 class TimerCallback {
 public:
-    enum SignalResult {
-        WAITING     = 0,
-        TIMEOUT     = 1,
-        EXIT_LOOP   = 2
-    };
-
-    TimerCallback (void* pContext) 
-    : m_Context(pContext)
-    , m_bContinue(false)
-    , m_nDelay(0)
+    TimerCallback()
+    : m_nDelay(0)
     , m_nCount(0)
-    , m_current_state(WAITING)
+    , m_stopRequested(false)
     {
         LOG(eWarning, "Timer Created\n");
     };
@@ -74,50 +64,25 @@ class TimerCallback {
         LOG(eWarning, "Timer destroyed\n");
     }; 
 
-    void Signal (TimerCallback::SignalResult value)
-    {
-        std::unique_lock<std::mutex> lck(m_mtx);
-        m_current_state = value;
-        m_cv.notify_one();
-        lck.unlock();
-    }
-
-    SignalResult Wait(unsigned int time_in_seconds)
+    bool Wait(unsigned int time_in_seconds)
     {
-        TimerCallback::SignalResult result;
-        std::chrono::milliseconds ms(time_in_seconds * 1000);
-
         std::unique_lock<std::mutex> lck(m_mtx);
-
-        if(m_current_state == WAITING) {
-            if(m_cv.wait_for(lck, ms) == std::cv_status::timeout) {
-                result = TIMEOUT;
-            }
-            else {
-                result = m_current_state;
-            }
-        }
-        else {
-            // State change before the lock was called
-            result = m_current_state;
-        }
-        m_current_state = WAITING;
-
-        lck.unlock();
-
-        return result;
+        return m_cv.wait_for(lck, std::chrono::seconds(time_in_seconds), [this] {
+            return m_stopRequested;
+        });
     }
 
     void StopTask() {
-        LOG(eWarning, "Stoping Timer Task\n");
-        m_bContinue = false;
-        Signal(EXIT_LOOP);
+        LOG(eWarning, "Stopping Timer Task\n");
+        {
+            std::lock_guard<std::mutex> lck(m_mtx);
+            m_stopRequested = true;
+        }
+        m_cv.notify_one();
         return;
     };
 
-    bool Loop() {
-        bool bTimerContinue = true;
-
+    void Loop() {
         LOG(eTrace, "Timer Callback! m_nCount = %d m_nDelay = %d\n", m_nCount, m_nDelay);
 
         // Validate that threads in process are still active
@@ -136,35 +101,25 @@ class TimerCallback {
         else {
             LOG(eTrace, "Could not find Process ID %X from map of size %d for reporting\n", (uint32_t)getpid(), RDKPerf_GetMapSize());
         }
-
-        return bTimerContinue;
     }
 
     void Task() {
-        m_bContinue = true;
         LOG(eWarning, "Task Started\n");
-        while(m_bContinue == true) {
-            if(!Loop()) {
-                LOG(eWarning, "Timer loop signaled for Exit..\n");
-                m_bContinue = false;
-                break;
-            }
+        while(!Wait(0)) {
+            Loop();
             LOG(eTrace, "Task sleeping %d seconds\n", TIMER_INTERVAL_SECONDS);
-            SignalResult result = Wait(TIMER_INTERVAL_SECONDS);
-            if(result == EXIT_LOOP) {
+            if(Wait(TIMER_INTERVAL_SECONDS)) {
                 LOG(eWarning, "Exit task loop has been signaled\n");
+                break;
             }
         }
         LOG(eWarning, "Task Completed\n");
         return;
     };
 private:
-    void*       m_Context;
-    bool        m_bContinue; 
     uint32_t    m_nDelay;
     uint32_t    m_nCount;
-    // Timeout, signaling
-    TimerCallback::SignalResult m_current_state;
+    bool         m_stopRequested;
     std::mutex   m_mtx;
     std::condition_variable m_cv;
 };
@@ -193,7 +148,7 @@ static void PerfModuleInit()
     LOG(eWarning, "RDK performance process initialize %X named %s\n", getpid(), strProcessName);       
 
     RDKPerf_InitializeMap();
-    s_timer = new TimerCallback(NULL);
+    s_timer = new TimerCallback();
 
     if(s_thread == NULL) {
         s_thread = new std::thread(&TimerCallback::Task, s_timer);
@@ -222,24 +177,23 @@ static void PerfModuleTerminate()
     RDKPerf_ReportProcess(pID);
 #endif    
 
-    // Remove prosess from list
-    RDKPerf_RemoveProcess(pID);
-
     // Wait for timer thread cleanup
     if(s_thread != NULL && s_thread->joinable()) {
         LOG(eWarning, "Cleaning up timer thread\n");
         s_thread->join();
-
-        delete s_thread;
-        if(s_timer != NULL) delete s_timer;
-
-        s_thread = NULL;
-        s_timer = NULL;
     }
     else {
         LOG(eError, "Thread does not exist\n"); 
     }
 
+    delete s_thread;
+    delete s_timer;
+    s_thread = NULL;
+    s_timer = NULL;
+
+    // Remove process from list after the timer can no longer access it
+    RDKPerf_RemoveProcess(pID);
+
 #ifdef PERF_REMOTE
     if(s_pQueue != NULL) {
         s_pQueue->Release();

test/Makefile

@@ -37,6 +37,8 @@ NAME = perftest
 
 SRC_DIRS = .
 
+.PHONY: quick-exit-test clean
+
 DIR_CREATE = @mkdir -p $(@D)
 
 # Find all the C and C++ files we want to compile
@@ -55,6 +57,9 @@ $(BUILD_DIR)/%.cpp.o: %.cpp
 $(BUILD_DIR)/$(NAME): $(OBJS)
 	$(CC) $(CFLAGS) -o $@ $(OBJS) $(LD_FLAGS) 
 
+quick-exit-test:
+	sh ./timer_quick_exit_test.sh "$(BUILD_DIR)"
+
 clean:
 	rm -f $(OBJS)
 	rm -f $(BUILD_DIR)/$(NAME)

test/timer_quick_exit_test.sh

@@ -0,0 +1,26 @@
+#!/bin/sh
+
+set -eu
+
+build_dir=$1
+iterations=${2:-100}
+library="$build_dir/librdkperf.so"
+
+if [ ! -f "$library" ]; then
+    echo "Missing test library: $library" >&2
+    exit 1
+fi
+
+iteration=1
+while [ "$iteration" -le "$iterations" ]; do
+    if ! timeout 3s env \
+        LD_LIBRARY_PATH="$build_dir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" \
+        LD_PRELOAD="$library" \
+        /bin/true >/dev/null 2>&1; then
+        echo "Timer quick-exit test failed on iteration $iteration" >&2
+        exit 1
+    fi
+    iteration=$((iteration + 1))
+done
+
+echo "Timer quick-exit test passed ($iterations iterations)"