RDKEMW-16851: Increase Process name buffer length from 16 to 64#322
RDKEMW-16851: Increase Process name buffer length from 16 to 64#322shibu-kv merged 21 commits intosupport/1.8from
Conversation
#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>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
…-306 # Conflicts: # CHANGELOG.md Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the DCA process-usage collection logic to better handle longer process names and to make CPU/memory extraction from top output more robust.
Changes:
- Replace unbounded
memcpy(..., strlen()+1)with a bounded copy intopInfo.processName. - Clear existing
marker->cpuValue/marker->memValuebefore repopulating them. - Update
top | grep ...command construction to try matching either by process name or by PID, and make an existing debug log reachable by moving areturn.
| strncpy(pInfo.processName, processName, BUF_LEN - 1); | ||
| pInfo.processName[BUF_LEN - 1] = '\0'; |
There was a problem hiding this comment.
The PR title/description says the process name buffer is increased from 16 to 64, but this change still writes into pInfo.processName sized by BUF_LEN (which is currently 16 in source/dcautil/dcautil.h). As a result, longer process names will still be truncated here. If the intent is truly a 64-byte process name, the buffer size needs to be updated at the type/define level (or introduce a dedicated PROC_NAME_BUF_LEN) and ensure all related code uses that size consistently.
| inFp = v_secure_popen("r", "top -n 1 -c | grep -v grep | { grep -i '%s' || grep -E '^[[:space:]]*%d[[:space:]]'; }", pInfo->processName, pInfo->pid[0]); | ||
| #else | ||
| sprintf(command, "top -n 1 -c | grep -v grep |grep -i '%s'", pInfo->processName); | ||
| snprintf(command, CMD_LEN, "top -n 1 -c | grep -v grep | { grep -i '%s' || grep -E '^[[:space:]]*%d[[:space:]]'; }", pInfo->processName, pInfo->pid[0]); | ||
| inFp = popen(command, "r"); |
There was a problem hiding this comment.
The { grep -i ... || grep -E ...; } fallback in a pipeline won’t work as intended: the first grep consumes all stdin even when it finds no matches, so the second grep runs with EOF and cannot match by PID. If you need a name-or-PID match from a single top output stream, use a single filter (e.g., a single grep -E with alternation, or awk matching either $1==pid or the command column contains the name).
| { | ||
| #ifdef LIBSYSWRAPPER_BUILD | ||
| inFp = v_secure_popen("r", "top -n 1 -c | grep -v grep |grep -i '%s'", pInfo->processName); | ||
| inFp = v_secure_popen("r", "top -n 1 -c | grep -v grep | { grep -i '%s' || grep -E '^[[:space:]]*%d[[:space:]]'; }", pInfo->processName, pInfo->pid[0]); |
There was a problem hiding this comment.
This adds more advanced regex capabilities which is more oriented towards bash. This may not work on all limited busy box provided shell capabilities.
shibu-kv
left a comment
There was a problem hiding this comment.
Please refer to comments from previous review submission
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
| /* match by process name (case-insensitive), PID as fallback */ | ||
| if(strcasestr(top_op, pInfo->processName) == NULL) | ||
| { | ||
| int line_pid = 0; | ||
| if(sscanf(top_op, "%d", &line_pid) != 1 || line_pid != pInfo->pid[0]) |
There was a problem hiding this comment.
The new matching/parsing logic (case-insensitive name match with PID fallback, plus the direct-file read path) isn’t covered by existing unit tests (current tests only exercise the NULL-pInfo case for getCPUInfo). Adding gtest coverage for these new branches would help prevent regressions across different top output formats and ensure the fallback behavior works as intended.
There was a problem hiding this comment.
@copilot Create a separate pull request with adding additional L1 test cases .
| /* match by process name (case-insensitive), PID as fallback */ | ||
| if(strcasestr(top_op, pInfo->processName) == NULL) | ||
| { | ||
| int line_pid = 0; | ||
| if(sscanf(top_op, "%d", &line_pid) != 1 || line_pid != pInfo->pid[0]) |
There was a problem hiding this comment.
@copilot Create a separate pull request with adding additional L1 test cases .
support/1.8branchdcaproc.cis identical betweensupport/1.8.5andsupport/1.8support/1.8into branch to align historysupport/1.8shows onlydcaproc.cchanges (1 file, 21 insertions, 20 deletions)topic/BES1-941tosupport/1.8in GitHub UI (Edit → Base branch)