fix(apple): keep a view's service alive while the view is used - #652
Merged
Conversation
`odr::HtmlView` looks self-sufficient - it holds a `shared_ptr` - but the impl behind it, `internal::html::HtmlView`, keeps only a bare `abstract::HtmlService *` and forwards `config()` and `write_html()` through it. A view handed out by `-[ODRHtmlService views]` therefore dangles as soon as the service goes, and `try service().views.first` - a temporary service, which is how anyone would write it - segfaulted on both macOS and the simulator. `ODRHtmlView` now carries its `ODRHtmlService` the way `ODRElement` carries its `ODRDocument`. The other two bindings already do this: JNI passes an `owner` to the `NativeResource` constructor, python pins the service onto the view as `_service`. `testRenderedHtmlCarriesItsOwnStyles` has been red on main since it landed - it is the crash, not a flake. `testViewsKeepTheirServiceAlive` states the invariant directly, next to the element suite's equivalent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YSePnR4Jcj5cq3H1D11xcd
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.
🤖 Generated with Claude Code
odr::HtmlViewlooks self-sufficient — it holds ashared_ptr— but the impl behind it,internal::html::HtmlView, keeps only a bareabstract::HtmlService *and forwardsconfig()andwrite_html()through it. A view handed out by-[ODRHtmlService views]therefore dangles as soon as the service goes, andsegfaulted on both macOS and the simulator. That is ordinary Swift, not a contrived lifetime — the same call with the service in a local passes, which is why
testRendersHtmlwas green next to it.ODRHtmlViewnow carries itsODRHtmlServicethe wayODRElementalready carries itsODRDocument. The other two bindings solved this when they bound views: JNI passes anownerinto theNativeResourceconstructor, python pins the service onto the view as_servicewith the comment "translate(...).list_views()[0]must not dangle". Apple was the one that missed it.Why this is urgent
appleis the only red workflow on main, andrelease.ymlhasneeds: [version, apple]— a red test in the called workflow skips the release job, so no release can be cut until this lands.testRenderedHtmlCarriesItsOwnStyleshas been failing since it landed in #648: it was cancelled on that commit, first actually ran on #649, and failed. It has never been green on main. It is the crash, not a flake.Verification
Built both macOS slices locally, assembled the xcframework, ran the suite via
xcodebuild test:testRenderedHtmlCarriesItsOwnStylesowner:nilspliced back in —testRenderedHtmlCarriesItsOwnStylesand the newtestViewsKeepTheirServiceAliveboth segfault, whiletestRendersHtml/testBringOfflineWritesFiles(which hold the service in a local) still passSo the new test is load-bearing rather than decorative.
Also
apple/AGENTS.mdsaidodr::Elementwas the exception to "a wrapper holding ashared_ptris self-sufficient". It isn't any more, and that sentence is what makes this class of bug easy to reintroduce — updated, with the trap called out.