RDKEMW-16851: Fix for top log markers and the polling frequency #371
Open
yogeswaransky wants to merge 1 commit into
Open
RDKEMW-16851: Fix for top log markers and the polling frequency #371yogeswaransky wants to merge 1 commit into
yogeswaransky wants to merge 1 commit into
Conversation
* RDKEMW-16851: Increase Process name buffer length from 16 to 64 (#322) * DELIA-70280: Box boot up time is extended due to Privacy control hangs (#305) * RDKEMW-15927 ,RDKEMW-15007 : [APACA/Xione DE] rbus self-deadlock in XConf privacy mode fetch causing ~188s T2 init delay Reason for change: When UserSettings sets privacy mode to DO_NOT_SHARE during boot, the rbus SET handler (t2PropertyDataSetHandler) runs synchronously on the single rbus callback thread — executing deleteAllProfiles() with pthread_join, disk I/O, and XConf client restart. The restarted XConf thread calls appendRequestParams() which uses getParameterValue(PRIVACYMODES_RFC) to fetch privacy mode. This issues rbus_get() on T2's own bus handle, requiring the same rbus callback thread that is already blocked processing the SET handler. This creates a self-deadlock with a 15s rbus timeout per attempt, compounded by XCONF_RETRY_TIMEOUT (180s), stalling T2 initialization. Cascading effect: While T2 is stalled, external callers (e.g. WPEFramework SystemServices plugin) invoking t2_event_s() block 15s each on rbus_getUint(Telemetry.OperationalStatus), delaying the dispatch thread by ~188s and blocking sendNotify() for EVT_ONSYSTEMPOWERSTATECHANGED. Fix: - xconfclient.c: Replace getParameterValue(PRIVACYMODES_RFC) with direct getPrivacyMode() call from privacycontrol library. The platform implementation (provided via meta layer bbappend) reads from a local in-memory cache (PRIVACY_STATE), with fallback to Thunder JSON-RPC (local HTTP) and persistent storage — none of which use rbus, eliminating the self-deadlock. ` - xconf-client/Makefile.am: Add privacycontrol include path and build dependency when IS_PRIVACYCONTROL_ENABLED is set. - telemetry_busmessage_sender.c: Add fast-fail file marker check (T2_COMPONENT_READY) before rbus_getUint() in isCachingRequired() to avoid 15s blocking timeout when T2 is not yet ready. Test Procedure: please refer the ticket comments Risks: Medium Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com> * Addressed the L1 Test cases failure * RDKEMW-15927, RDKEMW-15007 : [APACA/Xione DE] Fix RBUS handler starvation and race condition on privacyModeVal Reason for change: When UserSettings sets privacy mode to DO_NOT_SHARE during boot, the rbus SET handler (t2PropertyDataSetHandler) runs synchronously on the single rbus callback thread — executing deleteAllProfiles() with pthread_join, disk I/O, and XConf client restart. The restarted XConf thread calls appendRequestParams() which uses getParameterValue(PRIVACYMODES_RFC) to fetch privacy mode. This issues rbus_get() on T2's own bus handle, requiring the same rbus callback thread that is already blocked processing the SET handler. This creates a self-deadlock with a 15s rbus timeout per attempt, compounded by XCONF_RETRY_TIMEOUT (180s), stalling T2 initialization. Additionally, privacyModeVal global pointer has no mutex protection, allowing use-after-free and NULL dereference under concurrent SET/GET operations during stress scenarios. Cascading effect: While T2 is stalled, external callers (e.g. WPEFramework SystemServices plugin) invoking t2_event_s() block 15s each on rbus_getUint(Telemetry.OperationalStatus), delaying the dispatch thread by ~188s and blocking sendNotify() for EVT_ONSYSTEMPOWERSTATECHANGED. Under concurrent privacy mode toggling, PROVIDER_NOT_RESPONDING with 26+ messages queued and dropped, telemetry events fall back to file caching. Fix: - rbusInterface.c: Add privacyModeMutex (PTHREAD_MUTEX_INITIALIZER) to serialize all reads/writes of privacyModeVal in SET and GET handlers, preventing use-after-free and NULL dereference. - rbusInterface.c: Add privacyModeCallbackWorker() detached thread. The SET handler now validates input, updates privacyModeVal under mutex, calls setPrivacyMode() for persistence, then dispatches heavy callbacks (mprofilesDeleteCallBack, privacyModesDoNotShareCallBack) to the worker thread. RBUS handler returns immediately, preventing handler thread starvation and cascading 180s timeout loops. - xconfclient.c: Replace getParameterValue(PRIVACYMODES_RFC) with direct getPrivacyMode() call from privacycontrol library. The platform implementation (provided via meta layer bbappend) reads from a local in-memory cache (PRIVACY_STATE), with fallback to Thunder JSON-RPC (local HTTP) and persistent storage — none of which use rbus, eliminating the self-deadlock. - xconf-client/Makefile.am: Add privacycontrol include path and build dependency when IS_PRIVACYCONTROL_ENABLED is set. Add else branch for libxconfclient_la_CFLAGS to define the variable in all automake conditions, fixing autoreconf failure. - telemetry_busmessage_sender.c: Add fast-fail file marker check (T2_COMPONENT_READY) before rbus_getUint() in isCachingRequired() to avoid blocking on rbus timeout when T2 is not ready. - persistence.c: Change rm -f to rm -rf in clearPersistenceFolder() non-LIBSYSWRAPPER path. PRIVACYMODE_PATH is a directory, not a file; rm -f fails silently leaving stale persistence data. Test Procedure: Concurrent stress test with multiple privacy mode toggles (SHARE/DO_NOT_SHARE via curl JSON-RPC), telemetry marker submissions (telemetry2_0_client), rbuscli reads, and SIGUSR1 signal — all running simultaneously in background. Risks: Medium Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com> * Addressed the shibu review comments * Addressed the empty check on path on fetchLocalConfigs and clearPersistenceFolder * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Removed the extra space for addressed the style formatting * Addressed the astyle format issue * Fix dangling pointer in rbusInterface.c GET handler: call rbusValue_SetString after ownership transfer Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/d5b743ff-3289-4141-b51a-399971064c4c Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com> --------- Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com> Co-authored-by: tabbas651 <thamimrazith_abbasali@comcast.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Changelog updates for hotfix 1.8.5v1 * BES1-941: Update dcautil.h * Update dcaproc.c * Update dcaproc.c * Update dcaproc.c * Update dcaproc.c * Update dcaproc.c * BES1-941: Update dcaproc.c * BES1-941: Update dcaproc.c * BES1-941: Update dcaproc.c * BES1-941: Update reportgen.c * BES1-941: Update dcaproc.c * RDKEMW-16851: Update dcautil.h * RDKEMW-16851: Update dcaproc.c * RDKEMW-16851: Update dcaproc.c * RDKEMW-16851: Revert Info log lines to debug log lines Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * Initial plan * RDKEMW-16851: Update dcaproc.c Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Update dcaproc.c Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> --------- Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com> Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> Co-authored-by: Shibu Kakkoth Vayalambron <shibu.kakkoth@gmail.com> Co-authored-by: tabbas651 <thamimrazith_abbasali@comcast.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Yogeswaran K <166126056+yogeswaransky@users.noreply.github.com> Co-authored-by: Yogeswaran K <yogeswaransky@gmail.com> Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers (#332) * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKEMW-16851: Fix for skipfrequency issue in top markers Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> --------- Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com> * RDKEMW-16851: Fix polling frequency for Load Average (#335) Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com> * RDKEMW-16851: Update L1-tests.yml * RDKEMW-16851: Update L2-tests.yml * fix: eliminate TOCTOU race in getCPUInfo by replacing access()+fopen() with fopen() alone (CWE-367) Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/c6e1f474-c6d8-4cab-8c47-ba6cc59ed427 Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com> * RDKEMW-16851: Update dcautilTest.cpp --------- Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com> Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Shibu Kakkoth Vayalambron <shibu.kakkoth@gmail.com> Co-authored-by: tabbas651 <thamimrazith_abbasali@comcast.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Yogeswaran K <166126056+yogeswaransky@users.noreply.github.com> Co-authored-by: Yogeswaran K <yogeswaransky@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates DCA top-marker processing to better handle saved top logs, process-name buffer sizing, and related unit test expectations.
Changes:
- Increases process-name buffer length for DCA process info.
- Refactors top CPU parsing to read saved top output directly and filter lines in-process.
- Clears stale top marker values before processing and adjusts associated unit test/mocking behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
source/dcautil/dcautil.h |
Expands BUF_LEN used by process info fields. |
source/dcautil/dcaproc.c |
Updates process-name copying and CPU/top output parsing behavior. |
source/dcautil/dca.c |
Clears previous top marker values before skip/processing logic. |
source/test/dcautils/dcautilTest.cpp |
Updates DCA unit test expectations for the revised top parsing flow. |
source/test/mocks/FileioMock.cpp |
Adds mock-instance tracking to reset fscanf call state. |
Comments suppressed due to low confidence (1)
source/dcautil/dcaproc.c:615
- The PID fallback only compares the current top line against
pInfo->pid[0].getProcUsagecan populate multiple PIDs for the same process andgetMemInfosums all of them, so CPU usage can undercount when a top line does not contain the process name but matches one of the later PIDs. Compareline_pidagainst every entry inpInfo->pidup topInfo->total_instancebefore skipping the line.
int line_pid = 0;
if(pInfo->pid == NULL || sscanf(top_op, "%d", &line_pid) != 1 || line_pid != pInfo->pid[0])
{
Comment on lines
+148
to
+150
| if (g_fileIOMock != last_mock) { | ||
| call_count = 0; | ||
| last_mock = g_fileIOMock; |
Comment on lines
+594
to
+596
| int line_pid = 0; | ||
| if(pInfo->pid == NULL || sscanf(top_op, "%d", &line_pid) != 1 || line_pid != pInfo->pid[0]) | ||
| { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.