chore: auto-sync develop → main (direct push failure) - #111
Closed
brendanobra wants to merge 14 commits into
Closed
Conversation
Chore: one time sync of main -> develop to prepare for ff automation
RDKEMW-19316 : Add coding guidelines for firebolt-cpp-transport
…cation * Replace SubscriptionData value map with shared_ptr; drop stale notifications via weak_ptr * Address copilot comments * Address copilot comments * Catch any_cast errors in onPropertyChangedCallback * Address copilot comments
RDKEMW-22177: update automation to use new PAT
Contributor
There was a problem hiding this comment.
Pull request overview
Sync-related conflict resolution PR that also brings in the develop → main changeset, including a fix for a subscription-notification race (MONUI-908) by guarding SubscriptionData lifetime and hardening notification dispatch against exceptions.
Changes:
- Fix potential use-after-free during async event delivery by storing
SubscriptionDatainshared_ptrand wrapping subscription callbacks with aweak_ptrguard (HelperImpl::subscribe). - Prevent notification worker thread termination by catching exceptions (including
std::bad_any_cast) per-callback in the gateway notification worker. - Add regression tests covering the race scenario and callback-exception behavior; update GitHub workflows to use
AUTOMATION_TOKENand move/replace Copilot guidance with a dedicated coding-guidelines instructions file.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/helpers_impl.h |
Switch subscriptions storage to shared_ptr and wrap callbacks with weak_ptr to drop in-flight notifications safely after unsubscribe. |
include/firebolt/helpers.h |
Improve robustness of onPropertyChangedCallback by handling std::bad_any_cast explicitly and tightening log message formatting. |
src/gateway.cpp |
Wrap notification callback dispatch in try/catch to avoid std::terminate on callback exceptions. |
test/unit/helperTest.cpp |
Add regression tests for weak_ptr-guarded subscription callbacks covering unsubscribe races. |
test/unit/gatewayTest.cpp |
Add regression test ensuring notification worker continues after a callback throws. |
.github/workflows/sync-develop-to-main.yml |
Replace token secret usage with AUTOMATION_TOKEN for checkout/PR creation. |
.github/workflows/release.yml |
Replace token secret usage with AUTOMATION_TOKEN for checkout/semantic-release. |
.github/instructions/coding-guidelines.instructions.md |
Add comprehensive coding guidelines instruction file (applied to C++/CMake). |
.github/copilot-instructions.md |
Remove prior Copilot instructions file (superseded by new instructions). |
Comment on lines
+82
to
84
| void* notificationPtr = static_cast<void*>(it->second.get()); | ||
| auto errorStatus{gateway_.unsubscribe(it->second->eventName, notificationPtr)}; | ||
| subscriptions_.erase(it); |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Automatic sync of
develop→maincould not complete via direct workflow push.Action required: resolve the conflicts in this PR, then merge it.
Detected reason: direct push failure
Triggered by: daa7cb7 on develop