Only drop channel handlers if throwing an exception in case of declaration/get failure#731
Merged
milyin merged 1 commit intoeclipse-zenoh:mainfrom Feb 12, 2026
Conversation
|
PR missing one of the required labels: {'enhancement', 'internal', 'bug', 'breaking-change', 'documentation', 'dependencies', 'api-sync', 'new feature', 'ci'} |
c3d28b2 to
456e634
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical resource leak in channel handler cleanup when Zenoh API operations fail. The __ZENOH_RESULT_CHECK macro throws exceptions in exception mode (when err == nullptr), which would previously skip the cleanup code that followed it, causing handler leaks. Additionally, the PR corrects an inconsistent double-move pattern in two locations.
Changes:
- Moves handler cleanup to occur before
__ZENOH_RESULT_CHECKcalls, conditional onerr == nullptrto prevent resource leaks when exceptions are thrown - Fixes inconsistent
::z_move(*as_moved_c_ptr(...))double-move pattern to use correctas_moved_c_ptr(...) - Ensures error code mode (
err != nullptr) returns handlers to callers for proper management
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| include/zenoh/api/session.hxx | Updated 5 channel handler cleanup locations in get, declare_queryable, declare_subscriber, liveliness_declare_subscriber, and liveliness_get |
| include/zenoh/api/querier.hxx | Updated 2 channel handler cleanup locations in Querier::get and declare_matching_listener |
| include/zenoh/api/publisher.hxx | Updated channel handler cleanup in Publisher::declare_matching_listener |
| include/zenoh/api/ext/session_ext.hxx | Updated 2 channel handler cleanup locations in declare_querying_subscriber and declare_advanced_subscriber |
| include/zenoh/api/ext/advanced_subscriber.hxx | Updated channel handler cleanup in AdvancedSubscriberBase::detect_publishers, fixed double-move pattern |
| include/zenoh/api/ext/advanced_publisher.hxx | Updated channel handler cleanup in AdvancedPublisher::declare_matching_listener |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
milyin
approved these changes
Feb 12, 2026
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.
Only drop channel handlers if throwing an exception in case of declaration/get failure