chore: auto-sync develop → main (direct push failure) - #112
Closed
brendanobra wants to merge 15 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
This PR is an automated develop → main sync PR created after a direct workflow push failure, bringing in the latest changes including a subscription-notification race fix and related regression tests, plus workflow token updates and Copilot instruction restructuring.
Changes:
- Add weak_ptr-based guarding for subscription callback dispatch to prevent use-after-free during unsubscribe races.
- Add regression tests for the race condition and for notification-worker exception containment.
- Update GitHub Actions workflows to use
secrets.AUTOMATION_TOKEN, and replace.github/copilot-instructions.mdwith a repo-scoped instructions file.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/helpers_impl.h |
Store subscriptions as shared_ptr<SubscriptionData> and wrap event callbacks with a weak_ptr guard to safely drop in-flight notifications after unsubscribe. |
include/firebolt/helpers.h |
Adjust onPropertyChangedCallback to handle std::bad_any_cast and improve log message formatting. |
src/gateway.cpp |
Prevent notification worker thread termination by catching exceptions per-callback during dispatch. |
test/unit/helperTest.cpp |
Add regression tests validating notification delivery while subscribed and safe dropping after unsubscribe/unsubscribeAll. |
test/unit/gatewayTest.cpp |
Add regression test ensuring notification worker continues dispatch after a callback throws. |
.github/workflows/sync-develop-to-main.yml |
Switch workflow authentication to secrets.AUTOMATION_TOKEN for sync/push/PR creation steps. |
.github/workflows/release.yml |
Switch release workflow authentication to secrets.AUTOMATION_TOKEN. |
.github/instructions/coding-guidelines.instructions.md |
Add consolidated coding guidelines for the repository. |
.github/copilot-instructions.md |
Remove legacy Copilot instructions file (superseded by the new instructions document). |
Comment on lines
+82
to
85
| void* notificationPtr = static_cast<void*>(it->second.get()); | ||
| auto errorStatus{gateway_.unsubscribe(it->second->eventName, notificationPtr)}; | ||
| subscriptions_.erase(it); | ||
| return Result<void>{errorStatus}; |
Comment on lines
62
to
66
| catch (const std::exception& e) | ||
| { | ||
| FIREBOLT_LOG_ERROR("Event", "Cannot parse event data for event %s, payload: %s", | ||
| FIREBOLT_LOG_ERROR("Event", "Cannot parse event data for event '%s', payload: %s", | ||
| subscriptionData->eventName.c_str(), jsonResponse.dump().c_str()); | ||
| } |
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