Skip to content

Feature/rdkemw 9528 #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
madhubabutt wants to merge 6 commits into
base: feature/test
Choose a base branch
from
Open

Feature/rdkemw 9528 #250

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6af91a8
RDKEMW-9528: Align the sequence asynchronous rbus call
shibu-kv Oct 21, 2025
ceedca5
RDKEMW-9528: Align the sequence asynchronous rbus call
yogeswaransky Nov 3, 2025
f479163
RDKEMW-9528: Align the sequence asynchronous rbus call
yogeswaransky Nov 3, 2025
ecf785d
RDKEMW-9528: Align the sequence asynchronous rbus call
yogeswaransky Nov 3, 2025
87aaef0
RDKEMW-9528: Align the sequence asynchronous rbus call
Nov 7, 2025
f5dcd39
RDKEMW-9528: Align the sequence asynchronous rbus call
Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/ccspinterface/rbusInterface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ void publishReportUploadStatus(char* status)
rbusValue_Init(&value);
rbusValue_SetString(value, status);
rbusObject_SetValue(outParams, "UPLOAD_STATUS", value);
rbusValue_Release(value); // FIX: Release value immediately after SetValue to prevent double-free
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "Release value immediately after SetValue to prevent double-free" but this is misleading. There is no double-free issue in the original code - each resource was released exactly once. The actual issue with this change is that releasing the value too early may cause rbusMethod_SendAsyncResponse to reference freed memory, since rbusObject_SetValue likely does not create a copy of the value. The comment should be removed or corrected.

Suggested change
rbusValue_Release(value); // FIX: Release value immediately after SetValue to prevent double-free
rbusValue_Release(value); // Release local reference; outParams now holds its own reference to this value

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Releasing the value immediately after rbusObject_SetValue may be premature. Throughout the codebase (lines 959-972, 1000-1016, 1130-1151), the consistent pattern is to release the value AFTER the object containing it has been used (e.g., after rbusMethod_SendAsyncResponse or rbusEvent_Publish). This is likely because rbusObject_SetValue may not create a copy of the value, so releasing it early could cause the object to reference freed memory. The original code that released value after SendAsyncResponse (at line 909 old) was correct.

Copilot uses AI. Check for mistakes.

T2Info("Sending response as %s from %s \n", status, __FUNCTION__ );
err = rbusMethod_SendAsyncResponse(onDemandReportCallBackHandler, RBUS_ERROR_SUCCESS, outParams);
Expand All @@ -904,9 +905,8 @@ void publishReportUploadStatus(char* status)
T2Info("%s rbusMethod_SendAsyncResponse sent successfully \n", __FUNCTION__);
}
onDemandReportCallBackHandler = NULL; // just a safety cleanup
pthread_mutex_unlock(&asyncMutex);
rbusValue_Release(value);
rbusObject_Release(outParams);
rbusObject_Release(outParams); // FIX: Only release outParams, value is already released above
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Only release outParams, value is already released above" is correct but reinforces an incorrect change. The value should not have been released early (see comment on line 895).

Copilot uses AI. Check for mistakes.
pthread_mutex_unlock(&asyncMutex); // FIX: Unlock mutex after cleanup to prevent race conditions
Comment on lines +908 to +909
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Unlock mutex after cleanup to prevent race conditions" is incorrect. Moving the mutex unlock after cleanup does not prevent race conditions; instead, it unnecessarily extends the critical section. The mutex protects the shared variable onDemandReportCallBackHandler (set at line 944, used at line 898, cleared at line 907). Once that variable is cleared at line 907, the critical section should end and the mutex should be unlocked. The resource cleanup operations don't need mutex protection. The original ordering (unlock before cleanup) was correct.

Suggested change
rbusObject_Release(outParams); // FIX: Only release outParams, value is already released above
pthread_mutex_unlock(&asyncMutex); // FIX: Unlock mutex after cleanup to prevent race conditions
pthread_mutex_unlock(&asyncMutex); // Unlock once shared handler is cleared
rbusObject_Release(outParams); // FIX: Only release outParams, value is already released above

Copilot uses AI. Check for mistakes.
Comment on lines +908 to +909
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mutex unlock has been moved to after resource cleanup, which unnecessarily extends the critical section. The mutex (asyncMutex) protects the shared state onDemandReportCallBackHandler, which is already set to NULL at line 907. The resource cleanup operations (rbusObject_Release) do not need mutex protection and should not be performed while holding the lock. This increases lock contention and could impact performance. The mutex should be unlocked immediately after line 907, before the cleanup operations, as it was in the original code.

Suggested change
rbusObject_Release(outParams); // FIX: Only release outParams, value is already released above
pthread_mutex_unlock(&asyncMutex); // FIX: Unlock mutex after cleanup to prevent race conditions
pthread_mutex_unlock(&asyncMutex);
rbusObject_Release(outParams); // FIX: Only release outParams, value is already released above

Copilot uses AI. Check for mistakes.

T2Debug("--OUT %s\n", __FUNCTION__);

Expand Down
Loading