Replace SubscriptionData value map with shared_ptr; drop stale notifi…#105
Open
swethasukumarr wants to merge 3 commits into
Open
Replace SubscriptionData value map with shared_ptr; drop stale notifi…#105swethasukumarr wants to merge 3 commits into
swethasukumarr wants to merge 3 commits into
Conversation
…cations via weak_ptr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the C++ helper subscription path against a use-after-free race (MONUI-908) where in-flight notifications could invoke callbacks after SubscriptionData was destroyed.
Changes:
- Store
SubscriptionDataasstd::shared_ptrand guard event callbacks with astd::weak_ptrto safely drop stale notifications after unsubscribe. - Add regression unit tests covering notification delivery while subscribed and dropping after
unsubscribeAll()/unsubscribe(id). - Add exception handling in the gateway notification worker to prevent termination on callback exceptions (including
std::bad_any_cast).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/helpers_impl.h |
Switches subscription storage to shared_ptr and wraps gateway callbacks with a weak_ptr guard to avoid use-after-free. |
src/gateway.cpp |
Wraps notification callback dispatch in try/catch to log and continue on exceptions during notification delivery. |
test/unit/helperTest.cpp |
Adds regression tests to validate that stale notifications are dropped without crashing and that active subscriptions still receive notifications. |
swethasukumarr
force-pushed
the
feature/raceFix
branch
from
July 17, 2026 18:48
bb72ce6 to
f6a979c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
swethasukumarr
force-pushed
the
feature/raceFix
branch
from
July 17, 2026 19:46
f6a979c to
bf90634
Compare
swethasukumarr
force-pushed
the
feature/raceFix
branch
from
July 17, 2026 20:03
bf90634 to
2e330da
Compare
| ASSERT_TRUE(subResult); | ||
| ASSERT_TRUE(capturedCallback) << "wrappedCallback must have been captured"; | ||
|
|
||
| // Worker thread dispatches notification while subscription is still alive |
Comment on lines
+691
to
+693
| // Platform sends onSpeechInterrupted — notification was already queued in-flight | ||
| // Replay the race: call the wrappedCallback with the now-stale usercb address. | ||
| ASSERT_TRUE(capturedCallback) << "wrappedCallback must have been captured"; |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…cations via weak_ptr