Skip to content

fix(mobile): clear Ghostty layer callbacks before teardown - #9095

Closed
juliusmarminge wants to merge 1 commit into
mainfrom
codex/mobile-ghostty-layer-lifetime
Closed

fix(mobile): clear Ghostty layer callbacks before teardown#9095
juliusmarminge wants to merge 1 commit into
mainfrom
codex/mobile-ghostty-layer-lifetime

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 1, 2026

Copy link
Copy Markdown
Member

The vendored iOS Ghostty renderer can release its layer wrapper while UIKit still retains the underlying layer. A later Core Animation display pass can then call into a freed renderer context.

Backport Ghostty's callback-clearing fix, pin the existing custom-I/O source revision, and rebuild the device and simulator arm64 archives. The rebuild script now rejects source changes beyond the checked-in patch while allowing ignored build output, and it preserves the separately produced VT headers during archive refresh.

Native rebuild required: yes. Both checked-in iOS Ghostty archives changed.

Verification:

  • The original simulator archive retained nonzero callback and context pointers after teardown. The patched simulator archive cleared both pointers in 25 of 25 retained-layer lifecycles and survived 16 forced display passes per lifecycle.
  • Both rebuilt archives remain arm64 with an iOS 16 minimum and keep the baseline archive members and exported symbols.
  • Source-guard fixtures cover clean first application, an already-applied repeat, ignored build output, and rejection of extra tracked or untracked source files. Bash syntax and diff checks pass.

Verification used a focused native lifecycle harness, not the full T3 Code UI or a physical device.

Implemented with GPT-5.6 Sol xhigh in the Codex harness.


Note

Medium Risk
Changes native iOS terminal rendering teardown and vendored GhosttyKit binaries; incorrect callback handling could cause crashes or subtle rendering bugs on layer teardown.

Overview
Fixes a use-after-free in the vendored iOS Ghostty renderer when UIKit keeps the Metal layer after the wrapper is released: a later Core Animation display pass could still invoke a freed renderer via the layer callback.

The PR backports Ghostty’s upstream fix as scripts/libghostty-ios-patches/0001-clear-display-callback-before-layer-release.patch, which clears the display callback in IOSurfaceLayer.release before releasing the layer. Docs (README, THIRD_PARTY_NOTICES) now record the pinned fork revision and that patch.

build-libghostty-ios16.sh is tightened for reproducible rebuilds: it clones/checks out a pinned custom-I/O revision (default under ~/.cache/t3code), applies the patch idempotently, and fails if the checkout has extra tracked or untracked changes beyond that patch. The Zig build runs with -j1, and header sync uses rsync without --delete so separately vendored VT headers are not wiped when refreshing archives. Checked-in device/simulator GhosttyKit binaries are expected to be rebuilt with this flow.

Reviewed by Cursor Bugbot for commit 15472f6. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Clear displayCallback before releasing IOSurfaceLayer in Ghostty mobile build

  • Adds a patch (0001-clear-display-callback-before-layer-release.patch) that calls setDisplayCallback(null, null) before releasing the layer in IOSurfaceLayer.release, preventing a dangling callback during teardown.
  • Rebuilds the GhosttyKit.xcframework binaries with the patch applied and updates build docs.
  • Reworks build-libghostty-ios16.sh to pin a specific Ghostty fork revision into ~/.cache/t3code, apply the required patch automatically, and fail if the source tree has unexpected changes.
  • Risk: the build script now defaults GHOSTTY_SOURCE_DIR to the pinned cache path and compiles with -j1; existing local checkouts must be re-pinned or will fail validation.

Macroscope summarized 15472f6.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.2 KiB −243 B (−1.8%) 15.1 KiB
Codex Thread snapshot wire 6.9 KiB 6.9 KiB −6 B (−0.1%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.3 KiB −237 B (−3.5%) 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 55.5 KiB −1.5 KiB (−2.6%) 66.4 KiB
Codex Live turn messages 10 8 −2 (−20.0%) 21
Claude Total thread wire 13.2 KiB 13.5 KiB +215 B (+1.6%) 15.1 KiB
Claude Thread snapshot wire 6.9 KiB 6.9 KiB −1 B (−0.0%) 7.3 KiB
Claude Live turn WebSocket wire 6.4 KiB 6.6 KiB +216 B (+3.3%) 7.8 KiB
Claude Live turn WebSocket decoded 56.3 KiB 57.8 KiB +1.5 KiB (+2.7%) 66.4 KiB
Claude Live turn messages 8 10 +2 (+25.0%) 21

Baseline: cb00746 · PR result: 15472f6 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 15472f6

Macroscope's review found this PR approvable — This is a localized iOS Ghostty teardown bug fix: both vendored archives clear display callbacks before releasing retained layers, while the remaining changes make native rebuilds reproducible and document the patch. It does not add a capability, alter product defaults, or affect unrelated production paths.

You can add or adjust custom eligibility rules. Learn more.

@t3dotgg

t3dotgg commented Sep 4, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6 Astra (preview) responding on behalf of Theo

This was closed as part of an automated cleanup pass. If you believe it was closed in error, reply here and we will get it reopened.

Closing as superseded by #9155, which is merged into main. The current vendored Ghostty revision clears the display callback before releasing the layer, covering this backport. This PR's older binary archives must not replace the newer GhosttyKit build. The source patch and build-guard proposal remain available in this branch's history.

@t3dotgg t3dotgg closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants