From 4c0e9c787734cdfe72a787dff8fa92b3ab4f55c4 Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Sun, 27 Sep 2026 13:15:55 -0500 Subject: [PATCH 1/6] WNCXIONE-662 : Timer loop stuck Reason for change: Fix race condition where process terminate happens before the timer thread is started. Test Procedure: Test using gst-inspect Risks: None Signed-off-by: Douglas Adler --- rdkperf/rdk_perf.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/rdkperf/rdk_perf.cpp b/rdkperf/rdk_perf.cpp index 37bb00e..17231ef 100644 --- a/rdkperf/rdk_perf.cpp +++ b/rdkperf/rdk_perf.cpp @@ -62,7 +62,6 @@ class TimerCallback { TimerCallback (void* pContext) : m_Context(pContext) - , m_bContinue(false) , m_nDelay(0) , m_nCount(0) , m_current_state(WAITING) @@ -110,7 +109,6 @@ class TimerCallback { void StopTask() { LOG(eWarning, "Stoping Timer Task\n"); - m_bContinue = false; Signal(EXIT_LOOP); return; }; @@ -141,18 +139,17 @@ class TimerCallback { } void Task() { - m_bContinue = true; LOG(eWarning, "Task Started\n"); - while(m_bContinue == true) { + while(true) { if(!Loop()) { LOG(eWarning, "Timer loop signaled for Exit..\n"); - m_bContinue = false; break; } LOG(eTrace, "Task sleeping %d seconds\n", TIMER_INTERVAL_SECONDS); SignalResult result = Wait(TIMER_INTERVAL_SECONDS); if(result == EXIT_LOOP) { LOG(eWarning, "Exit task loop has been signaled\n"); + break; } } LOG(eWarning, "Task Completed\n"); @@ -160,7 +157,6 @@ class TimerCallback { }; private: void* m_Context; - bool m_bContinue; uint32_t m_nDelay; uint32_t m_nCount; // Timeout, signaling From 88758a3f15756fa15fc463d432da4769f5cf49a0 Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Sun, 27 Sep 2026 13:58:33 -0500 Subject: [PATCH 2/6] WNCXIONE-662 : Timer loop stuck Reason for change: Simplifed the timer loop and added some additional tests. Test Procedure: Test using make quick-exit-test Risks: None Signed-off-by: Douglas Adler --- Makefile | 5 ++ rdkperf/rdk_perf.cpp | 96 ++++++++++------------------------- test/Makefile | 5 ++ test/timer_quick_exit_test.sh | 26 ++++++++++ 4 files changed, 63 insertions(+), 69 deletions(-) create mode 100644 test/timer_quick_exit_test.sh diff --git a/Makefile b/Makefile index 249a960..8d514aa 100644 --- a/Makefile +++ b/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 \ diff --git a/rdkperf/rdk_perf.cpp b/rdkperf/rdk_perf.cpp index 17231ef..b2a920c 100644 --- a/rdkperf/rdk_perf.cpp +++ b/rdkperf/rdk_perf.cpp @@ -39,8 +39,6 @@ #include -#define REPORTING_INITIAL_COUNT 5000 -#define REPORTING_INTERVAL_COUNT 20000 #define TIMER_INTERVAL_SECONDS 10 #define MAX_DELAY 600 @@ -54,17 +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_nDelay(0) + TimerCallback() + : m_nDelay(0) , m_nCount(0) - , m_current_state(WAITING) + , m_stopRequested(false) { LOG(eWarning, "Timer Created\n"); }; @@ -73,49 +64,25 @@ class TimerCallback { LOG(eWarning, "Timer destroyed\n"); }; - void Signal (TimerCallback::SignalResult value) + bool Wait(unsigned int time_in_seconds) { std::unique_lock lck(m_mtx); - m_current_state = value; - m_cv.notify_one(); - lck.unlock(); - } - - SignalResult Wait(unsigned int time_in_seconds) - { - TimerCallback::SignalResult result; - std::chrono::milliseconds ms(time_in_seconds * 1000); - - std::unique_lock 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"); - Signal(EXIT_LOOP); + LOG(eWarning, "Stopping Timer Task\n"); + { + std::lock_guard 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 @@ -134,20 +101,14 @@ 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() { LOG(eWarning, "Task Started\n"); - while(true) { - if(!Loop()) { - LOG(eWarning, "Timer loop signaled for Exit..\n"); - 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; } @@ -156,11 +117,9 @@ class TimerCallback { return; }; private: - void* m_Context; 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; }; @@ -189,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); @@ -218,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(); diff --git a/test/Makefile b/test/Makefile index 107a292..0f535df 100644 --- a/test/Makefile +++ b/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) diff --git a/test/timer_quick_exit_test.sh b/test/timer_quick_exit_test.sh new file mode 100644 index 0000000..98f37b7 --- /dev/null +++ b/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)" \ No newline at end of file From 165ab2b6a20f5c9312dda6f7885541e5b2ffc151 Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Sun, 27 Sep 2026 14:08:08 -0500 Subject: [PATCH 3/6] RDKEMW-25025 : Add coverity yml files Reason for change: Follow directions from https://etwiki.sys.comcast.net/spaces/RDKDocumentation/pages/1589622522/Setting+up+Coverity+for+RDK-E+in+the+GitHub+Actions+workflow Test Procedure: None Risks: None Signed-off-by: Douglas Adler --- .github/workflows/coverity_full_scan.yml | 22 +++++++++++++ .../workflows/coverity_incremental_scan.yml | 31 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 .github/workflows/coverity_full_scan.yml create mode 100644 .github/workflows/coverity_incremental_scan.yml diff --git a/.github/workflows/coverity_full_scan.yml b/.github/workflows/coverity_full_scan.yml new file mode 100644 index 0000000..eb30b63 --- /dev/null +++ b/.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@develop + with: + branchName: ${{ github.ref_name }} + # Specify your actual build command here + buildCommand: sh build.sh + # 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 }} diff --git a/.github/workflows/coverity_incremental_scan.yml b/.github/workflows/coverity_incremental_scan.yml new file mode 100644 index 0000000..ea7a42c --- /dev/null +++ b/.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@develop + 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: sh build.sh + # 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 }} From c0071e29e8109f58a51b3c8902bb21e1dc66db47 Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Sun, 27 Sep 2026 16:30:55 -0500 Subject: [PATCH 4/6] Potential fix for pull request finding 'Full Coverity scan invokes nonexistent build.sh' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/workflows/coverity_full_scan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverity_full_scan.yml b/.github/workflows/coverity_full_scan.yml index eb30b63..c0f814e 100644 --- a/.github/workflows/coverity_full_scan.yml +++ b/.github/workflows/coverity_full_scan.yml @@ -11,7 +11,7 @@ jobs: with: branchName: ${{ github.ref_name }} # Specify your actual build command here - buildCommand: sh build.sh + buildCommand: make all # Specify any packages required for building your components. # Enable the customSetup and add the package names below. #customSetup: | From d115aeadd37b7cbea6bf3ddca4a0d29608a5e90a Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Sun, 27 Sep 2026 16:31:04 -0500 Subject: [PATCH 5/6] Potential fix for pull request finding 'Incremental Coverity scan invokes nonexistent build.sh' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/workflows/coverity_incremental_scan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverity_incremental_scan.yml b/.github/workflows/coverity_incremental_scan.yml index ea7a42c..00b5eb4 100644 --- a/.github/workflows/coverity_incremental_scan.yml +++ b/.github/workflows/coverity_incremental_scan.yml @@ -19,7 +19,7 @@ jobs: pullRequestNumber: ${{ github.event.inputs.pullRequestNumber || github.event.pull_request.number }} branchName: ${{ github.event.pull_request.base.ref }} # Specify your actual build command here - buildCommand: sh build.sh + buildCommand: make all # Specify any packages required for building your components. # Enable the customSetup and add the package names below. #customSetup: | From 7588729398abd8ec475627305108f95d5ed2a75f Mon Sep 17 00:00:00 2001 From: Douglas Adler Date: Mon, 28 Sep 2026 12:12:43 -0500 Subject: [PATCH 6/6] Fix coverity branch Signed-off-by: Douglas Adler --- .github/workflows/coverity_full_scan.yml | 2 +- .github/workflows/coverity_incremental_scan.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coverity_full_scan.yml b/.github/workflows/coverity_full_scan.yml index c0f814e..bf52699 100644 --- a/.github/workflows/coverity_full_scan.yml +++ b/.github/workflows/coverity_full_scan.yml @@ -7,7 +7,7 @@ on: jobs: call_coverity_full_scan: - uses: rdk-e/build_tools_workflows/.github/workflows/coverity_component_full_scan.yml@develop + 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 diff --git a/.github/workflows/coverity_incremental_scan.yml b/.github/workflows/coverity_incremental_scan.yml index 00b5eb4..b11c37a 100644 --- a/.github/workflows/coverity_incremental_scan.yml +++ b/.github/workflows/coverity_incremental_scan.yml @@ -14,7 +14,7 @@ on: jobs: call_coverity_incremental_scan: - uses: rdk-e/build_tools_workflows/.github/workflows/coverity_component_incremental_scan.yml@develop + 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 }}