Skip to content

RDKEMW-26150: Make app cache replacement thread-safe - #222

Open
Kishoredarmaradje wants to merge 11 commits into
mainfrom
topic/RDKEMW-26150-FullFix
Open

Kishoredarmaradje wants to merge 11 commits into
mainfrom
topic/RDKEMW-26150-FullFix

Conversation

@Kishoredarmaradje

Copy link
Copy Markdown

No description provided.

Copilot AI balanced review requested due to automatic review settings October 21, 2026 11:47
@Kishoredarmaradje
Kishoredarmaradje requested a review from a team as a code owner October 21, 2026 11:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Dependency revision changes and compatibility-patch removals require native-build integration validation before approval.

No open issues 🎉

What changed in this PR

Makes application-cache replacement thread-safe by locking cache operations and returning copied entries to readers.

Changes:

  • Adds synchronized replacement and copy-based lookups.
  • Adds source-level guardrail checks for cache locking and usage.
  • Updates build dependencies and revisions, removing compatibility patches.
File Description
tests/​test_security_cache.py Checks locking and copy-based lookup patterns.
server/​plat/​gdialobjCacheHelper.hpp Adds mutex protection, replacement, and copied reads.
server/​plat/​gdialappcache.cpp Uses replacement and copy-based lookup APIs.
build_dependencies.sh Updates packages and dependency revisions; removes patches.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 23, 2026 09:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Registration dispatch introduces a condition-variable lifetime race and can deadlock shutdown.

2 open issues

Review effort: Balanced

Comment thread server/gdialservice.cpp
Comment on lines +177 to +181
{
std::lock_guard<std::mutex> lock(dispatch->mutex);
dispatch->complete = true;
}
dispatch->condition.notify_one();
Comment thread server/gdialservice.cpp
g_source_attach(source, server_main_context);
g_source_unref(source);
std::unique_lock<std::mutex> lock(dispatch.mutex);
dispatch.condition.wait(lock, [&dispatch]{ return dispatch.complete; });
Copilot AI balanced review requested due to automatic review settings October 23, 2026 09:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Registration dispatch has unresolved lifetime and shutdown-synchronization risks.

2 open issues

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 27, 2026 11:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Newly rejected application names can cause a null-pointer dereference because the registration wrapper does not handle constructor failure.

3 open issues

Review effort: Balanced


GDialAppRegistry* gdial_app_registry_new (const gchar *app_name, const GList *app_prefixes, GHashTable *properties, gboolean is_singleton, gboolean use_additional_data, const GList *allowed_origins) {
g_return_val_if_fail(app_name != NULL, NULL);
g_return_val_if_fail(gdial_app_name_is_safe(app_name), NULL);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants