Sync with main#15
Conversation
* Test for crash on exit Signed-off-by: Douglas Adler <douglas.adler@yahoo.com> * Test for crash on exit Signed-off-by: Douglas Adler <douglas.adler@yahoo.com> --------- Signed-off-by: Douglas Adler <douglas.adler@yahoo.com>
Deploy cla action
There was a problem hiding this comment.
Pull request overview
Syncs the latency branch with main by updating RDKPerf module shutdown behavior and adding a CLA GitHub Actions workflow.
Changes:
- Adjusts
PerfModuleTerminate()to stop the timer task earlier, gates report printing on exit behind#if 0, and tweaks timer/thread cleanup. - Adds a new
.github/workflows/cla.ymlreusable-workflow caller to enforce CLA signature checks on PR activity and comments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
rdkperf/rdk_perf.cpp |
Updates termination/shutdown sequence for the timer thread and suppresses report printing on exit. |
.github/workflows/cla.yml |
Introduces a CLA workflow triggered on PR events and issue comments using an external reusable workflow and a PAT secret. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(s_timer != NULL) { | ||
| s_timer->StopTask(); | ||
| } |
There was a problem hiding this comment.
s_timer->StopTask() writes m_bContinue from a different thread than TimerCallback::Task() reads it, but m_bContinue is a plain bool with no synchronization. This introduces a data race (undefined behavior) on process termination. Consider making m_bContinue an std::atomic<bool> (or guarding reads/writes with the same mutex used for the condition variable) so StopTask/Task communicate safely.
| if(s_thread != NULL && s_thread->joinable()) { | ||
| LOG(eWarning, "Cleaning up timer thread\n"); | ||
| s_timer->StopTask(); | ||
| s_thread->join(); | ||
|
|
||
| delete s_thread; | ||
| delete s_timer; | ||
| if(s_timer != NULL) delete s_timer; | ||
|
|
There was a problem hiding this comment.
Timer resource cleanup only happens inside the if (s_thread != NULL && s_thread->joinable()) branch. If s_thread is null / not joinable but s_timer was allocated (e.g., partial init, thread already detached, or future refactor), s_timer will leak. Consider deleting s_timer (and s_thread) independently of joinable() after ensuring the worker is stopped, or handling the else path to free any allocated objects.
|
|
||
| jobs: | ||
| CLA-Lite: | ||
| name: "Signature" |
There was a problem hiding this comment.
This workflow is triggered by issue_comment and pull_request_target while also passing a PAT secret (CLA_ASSISTANT) into a reusable workflow. In pull_request_target/issue_comment contexts, secrets are available even for forked PRs/comments, which increases the risk of secret exfiltration if the called workflow ever runs untrusted code or responds to attacker-controlled inputs. Consider restricting execution with a job-level if (e.g., only for PR comments, only specific comment commands, and/or only repository members) and minimizing permissions to the least required.
| name: "Signature" | |
| name: "Signature" | |
| if: >- | |
| github.event_name == 'pull_request_target' || | |
| ( | |
| github.event_name == 'issue_comment' && | |
| github.event.issue.pull_request && | |
| ( | |
| github.event.comment.author_association == 'OWNER' || | |
| github.event.comment.author_association == 'MEMBER' || | |
| github.event.comment.author_association == 'COLLABORATOR' | |
| ) | |
| ) |
| actions: write | ||
| statuses: write |
There was a problem hiding this comment.
The workflow requests broad permissions (actions: write, statuses: write, pull-requests: write). Unless the reusable CLA workflow strictly requires all of these, reduce permissions to least privilege to limit blast radius (especially since this is also triggered by issue_comment/pull_request_target).
| actions: write | |
| statuses: write |
Syncing the latency branch with main