Repository navigation
Fix message ownership, SWIG iterator safety, and container release recovery - #1667
Merged
Merged
Conversation
Track the owning module on borrowed output proxies and resolve that owner when redirecting another output. Retain the module independently for the redirected message and its consumers, including after the source output is rerouted. Reject saved proxies whose owning module has already been deleted. Avoid retaining the enclosing module when redirecting its own outputs. Add regression coverage for source-module deletion, unrelated-module writes, reader/recorder/payload lifetimes, reassignment, stale proxies, and direct or indirect redirection between co-owned outputs. Add patch release notes and a known-issue follow-up. Validation: the primary regression failed against the original backport. Rebuilt the isolated 2.12.2 Release/OpNav tree; 251 Python tests and 193 native tests passed. One MuJoCo-only test skipped because MuJoCo is disabled. The messaging header compiled in isolation, and the added RST parsed cleanly. Generated SWIG wrappers retain the pre-existing unused-owner warnings.
- Preserve module owners through vector access and iterator operations, including after late module imports. - Keep the GIL during unsafe native iterator operations. - Fix import-order-dependent numeric-vector bindings. - Add regression coverage and document message lifetime limitations. - Preserve the native ABI. Validation: 639 Python tests passed, 1 skipped; 193 native tests passed. Vizard OpNav completed with 200 camera frames and seven plots.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37944c3747
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Free hosted-runner disk before Docker setup, and allow manual recovery from an existing release tag while pinning both builds to its commit. Validate the source version, published images, and both manifest architectures. Make latest updates explicit for manual recovery. Validation: actionlint, pre-commit, shell/Python syntax, tag and version checks, and complete/incomplete manifest cases.
Follow _swig_bsk_owner chains when resolving message storage owners, so temporary MuJoCo body and site proxies do not cause false ReferenceError failures. Resolve destination owners to prevent self-retention when redirecting outputs within the same scene. Add regression coverage for temporary accessor chains, scene retention and release, and deleted-scene rejection. Update the release note.
schaubh
force-pushed
the
feature/msg_fixes
branch
from
November 3, 2026 01:01
227277a to
531a6e9
Compare
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.
Description
Port three fixes from the v2.12 maintenance branch to v2.13, preserving the existing ABI:
Raw pointer-vector copies and slices intentionally cannot establish ownership for redirection. Existing message-lifetime restrictions and image-buffer behavior remain unchanged.
The commits retain their original separation. Port-specific adjustments update the v2.13 template-module API usage and documentation snippet layout.
Verification
Added and expanded regression tests for redirected-output lifetimes, vector ownership, import-order behavior, iterator safety, and numeric-vector bindings.
Local validation passed:
Cross-platform execution and actual container publication have not yet been validated.
Documentation
Updated release-note and known-issue snippets describing the ownership fixes, iterator and numeric-vector corrections, and container release recovery.
Reviewers should check the documented restrictions on redirection order, native ownership cycles, pointer-vector copies and slices, and image-buffer lifetime. API comments also document the enclosing-owner argument used to prevent self-retention.
Future work
Broader ownership changes requiring an ABI break remain separate from this PR.