Skip to content

Revert portal hierarchy mutation tracker launch crash - #10018

Merged
austinywang merged 2 commits into
mainfrom
issue-10016-revert-portal-tracker-launch-crash
Aug 12, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-10016-revert-portal-tracker-launch-crash

Conversation

@austinywang

@austinywang austinywang commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #10016

Crash and rationale

Nightly 0.64.22-nightly.3154007367401, built from 71d80fa, crashed about ten seconds after launch on the main thread with EXC_BAD_ACCESS at address 0. The stack entered cmux from SwiftUICore _AppKitAddSubview during DisplayList rendering and reached objc_setAssociatedObject with a nil receiver.

The crashing nightly already included #10008, which moved insertion tracking before AppKit consumed the inserted view. Because that forward fix did not eliminate the launch crash, this PR follows the issue directive and removes the entire regression source instead of attempting a second nil/lifetime patch at a process-wide AppKit swizzle boundary.

Verification

  • Tagged cloud build succeeded from the committed PR HEAD with:
    CMUX_SKIP_ZIG_BUILD=1 /Users/austinwang/manaflow/cmuxterm-hq/scripts/reload-cloud.sh --tag issue-10016-revert-portal-tracker-launch-crash --launch
  • Fleet build run issue-10016-revert-portal-tracker-launch-crash-a35df7eebe94 completed in 247 seconds and launched the isolated bundle.
  • The launched process (PID 33372) remained alive and socket-responsive for at least 5m37s, far beyond the approximately ten-second crash window.
  • Exercised SwiftUI/AppKit hierarchy churn: created a second workspace, switched workspaces twice, created/resized/closed a horizontal split, created/resized/closed a vertical pane, and closed the temporary workspace.
  • Performed a real pointer drag on the horizontal divider. The tagged debug log recorded divider.session.mouseDown, divider.dragUpdate, and divider.dragEnd; pane widths changed from 459.5/299.5 to 484.5/274.5, confirming restored split-divider hit-testing still works.
  • No new cmux*.ips or cmux*.crash appeared in ~/Library/Logs/DiagnosticReports after the verification marker.
  • scripts/check-pbxproj.sh, scripts/lint-pbxproj-test-wiring.sh (671 files), scripts/check-package-resolved-policy.py, and git diff --check pass.
  • Swift warning audit reports zero cmux-owned warnings. No warning or file-length budget TSV was changed; scripts/swift_file_length_budget.py is not present on this branch.
  • Per task instructions, no local xcodebuild test or XCUITest suite was run; CI owns the test suites.

Summary by cubic

Reverts the portal hierarchy mutation tracker and AppKit swizzles to fix a launch-time EXC_BAD_ACCESS and restores the previous split-divider cache policy keyed by root subview identity. Addresses #10016 and keeps divider hit-testing working.

Written for commit 127dcc7. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Performance

    • Improved split-divider hit testing by streamlining cache updates and reducing unnecessary hierarchy checks.
    • Divider geometry caches now refresh more efficiently when relevant view structure or layout changes.
  • Bug Fixes

    • Improved cache reliability after view reordering, sorting, detachment, and other structural changes.
    • Reduced the risk of stale divider regions during window layout updates.
  • Tests

    • Simplified performance coverage while retaining validation that unrelated changes do not rebuild cached divider geometry.

@austinywang
austinywang merged commit 407a935 into main Aug 12, 2026
3 of 5 checks passed
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8affde43-af24-4a43-9094-9ac583605db9

📥 Commits

Reviewing files that changed from the base of the PR and between 5ff45e3 and 127dcc7.

📒 Files selected for processing (10)
  • Sources/BrowserWindowPortal.swift
  • Sources/PortalSplitDividerCacheInvalidator.swift
  • Sources/PortalSplitDividerRegion.swift
  • Sources/PortalViewHierarchyMutationRegistration.swift
  • Sources/PortalViewHierarchyMutationTracker.swift
  • Sources/PortalViewHierarchyNodeState.swift
  • Sources/TerminalWindowPortal.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/PortalDetachedHierarchyMutationTests.swift
  • cmuxTests/PortalHitTestingPerformanceTests.swift

📝 Walkthrough

Walkthrough

The split-divider cache now tracks root subview identities and observes geometry and structure views directly. Hierarchy mutation swizzling, registration, tracking state, and related tests were removed. Interaction-layer reordering always invokes sortSubviews.

Changes

Split-divider cache redesign

Layer / File(s) Summary
Direct view observation model
Sources/PortalSplitDividerRegion.swift, Sources/PortalSplitDividerCacheInvalidator.swift
Collection separates geometry views from structure views. The invalidator deduplicates both sets and observes subviews changes with KVO.
Cache identity and invalidation integration
Sources/BrowserWindowPortal.swift, Sources/TerminalWindowPortal.swift
Cache reuse compares root subview identities and live regions. Cache collection uses geometry and structure observations. Invalidation clears the identity snapshots. Interaction-layer reordering always calls sortSubviews.
Hierarchy tracker removal and test cleanup
Sources/PortalViewHierarchyMutationRegistration.swift, Sources/PortalViewHierarchyMutationTracker.swift, Sources/PortalViewHierarchyNodeState.swift, cmux.xcodeproj/project.pbxproj, cmuxTests/PortalDetachedHierarchyMutationTests.swift, cmuxTests/PortalHitTestingPerformanceTests.swift
Hierarchy mutation tracking and detached-hierarchy tests are removed. The Xcode project no longer references the deleted sources and tests. Performance tests retain unrelated mutation and cached-geometry assertions while removing hierarchy-tracking scenarios.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • manaflow-ai/cmux#8580: Introduced the portal hierarchy mutation tracking and cache invalidation mechanisms replaced by this change.

Suggested reviewers: ejc3, azooz2003-bit, lawrencecchen

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-10016-revert-portal-tracker-launch-crash

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

azooz2003-bit added a commit that referenced this pull request Aug 13, 2026
* Hide current iOS Agent GUI

* Revert portal hierarchy mutation tracker launch crash (#10018)

* Revert "Fix portal mutation tracker launch crash (#10008)"

This reverts commit a161d16.

* Revert "Portal: invalidate the split-divider hit-test cache on nested subview insertion (#8580)"

This reverts commit bcfb2d7.

---------

Co-authored-by: Austin Wang <austinwang115@gmail.com>
@austinywang austinywang mentioned this pull request Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant