Fix class availability watcher timing#45
Conversation
|
Codex review: found issues before merge. Reviewed June 11, 2026, 6:32 AM ET / 10:32 UTC. Summary Reproducibility: yes. at source/probe level: current master installs the watcher from Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the watcher timing fix after correcting legacy Mach-O section detection and letting the normal and forced-legacy runtime probes pass on the declared Apple platform matrix. Do we have a high-confidence way to reproduce the issue? Yes, at source/probe level: current master installs the watcher from Is this the best way to solve the issue? No, not quite yet. Moving the primary path to Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against fdea299f53da. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebe65e8385
ℹ️ 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".
| #if __LP64__ | ||
| const struct mach_header_64 *header64 = (const struct mach_header_64 *)header; |
There was a problem hiding this comment.
Detect Mach-O width from the loaded header
On watchOS 5 arm64_32, __LP64__ is false even though the loaded images use 64-bit Mach-O headers. That sends the legacy dyld fallback through the 32-bit getsectiondata path, so it can miss __objc_classlist, skip the Objective-C registration wait, and fire the watcher before NSClassFromString can see the class. Since watchOS 5 is still a declared deployment target and this fallback is specifically for pre-watchOS 6, branch on header->magic/MH_MAGIC_64 rather than the compile-time pointer model.
Useful? React with 👍 / 👎.
Summary
Root cause
_dyld_register_func_for_add_imageruns before Objective-C has registered classes from the image. The previous watcher queried immediately in that callback, missed the class, and did not retry until an unrelated later image load.A macOS arm64 probe reproduced the ordering as
callback=missing after_dlopen=visible. The new implementation usesobjc_addLoadImageFuncwhere available and a bounded Objective-C registration check for older supported systems.Verification
bash Tests/verify_class_load_watcher.shIKT_FORCE_DYLD_IMAGE_CALLBACK=1 bash Tests/verify_class_load_watcher.shbash Tests/verify_release_linking.shswift testandswift test -c release: 16 tests passed in each configurationLocal tvOS verification was unavailable because this Xcode installation has no tvOS runtime; GitHub Actions retains the tvOS job.
Closes #26