Skip to content

fix(next): retire browser pages before chrome teardown - #27166

Merged
austinywang merged 9 commits into
feat-cmux-nextfrom
feat-cmux-next-nightly-sigabrt-view-teardown
Oct 29, 2026
Merged

austinywang merged 9 commits into
feat-cmux-nextfrom
feat-cmux-next-nightly-sigabrt-view-teardown

Conversation

@austinywang

@austinywang austinywang commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Releasing hidden browser chrome could abort in Chromium’s AppKit hierarchy tracker: a descendant registered a weak reference to an ancestor already deallocating. BrowserEntry now closes the page and detaches its content before releasing chrome. Final release off main retains both objects through main-actor cleanup; explicit close uses the same retirement path.

The reported crash was matched by executable UUID to nightly commit 1f0f687b2f32a9f506e19e7f683e8362fd3da6b2. The crash marker’s newer version described the on-disk update. A live isolated baseline reproduced the same SIGABRT stack: weak_register_no_lock → objc_storeWeak → NSPointerArray.addPointer → Chromium hidden-ancestor tracking → NSView finalization.

Verification

Head: 83e5935206c0777a5d5ba2a4e67e7c53ec27ebc5.

Focused native command (run with each source SHA):

cmux-ci run --class light --script scripts/ci/package-test-lane.sh \
  --ref SHA --arg=suite --arg=Packages/macOS/CmuxNext \
  --arg=BrowserEntryRetirementTests \
  --env CMUX_CI_XCODE_APP=/Applications/Xcode_26.6.app
  • Before: test-only commit 76e2236c6806d386aff96e2e699f185a6c4c64f2, step 348a7122cf4795d1daac2f4e: failed as expected because the released owner left its page open (1 of 2 tests failed).
  • After: exact-head step f8ca670571d6b40dd99dca84: 3 tests passed, 0 skipped, exit 0 on Xcode 26.6. Covers hidden-owner release, repeated close, and final release off main with real WebKit/AppKit objects. The test process exited with no live test windows. Intermediate test compilation errors were repaired before this pass.
  • Exact-head tagged build: 3a1260654af924576823694f, tag 27166-browser-view-teardown: passed; downloaded artifact SHA-256 0782e3b69176e581324c0b78c349defd645ae1ca78d6a0c6f0e50d97c3e39e63, original bundle signature verified before isolated launch.
  • Live Chromium before: comparison commit dcc8aa3a1df9d67342947c9aadc222615f70d89d, build 9020003ddbefb27438b98c79, crashed on the first owner-release probe with the matching weak ancestor stack.
  • Live Chromium after: exact head/build above passed 10/10 owner-release probes. Every result reported live_before_release, closed, content_detached, chrome_released, and passed as true. The app stayed inactive, the frontmost application stayed the same, and the isolated app quit gracefully and exited afterward.
  • Swift syntax, prepush guardrails, and diff whitespace checks passed. The App source growth warning is advisory.

Observed / Not run

Observed: original crash and matching symbols; same-stack live baseline crash; 10 exact-head Chromium runs passing every lifecycle assertion; independent correctness review on this head. The native suite passed all 3 tests with no skips.

Not run: Sentry queries (CLI unauthenticated), a broad package sweep, and Cloud behavior (desktop lifecycle change only). Capture-service attempts had no available GUI runner; local verification uses separate tagged bundle IDs, data/configs, and control sockets with activation disabled.

No capture: no rendered UI change. Native runtime receipts and the baseline crash are retained privately.

Impact and review

BrowserEntry is the ownership boundary for both desktop browser engines. The retirement adapter lives in CmuxNextBrowser. The debug-only control endpoint exercises real CEF ancestor tracking; the helper remains available to the Browser module because the App’s bench flag does not propagate to it.

Review (review_browser_retirement): SHIP at 83e5935. Earlier review findings about macOS 14 Mutex support and module imports are resolved. Conflict-only merge gate passed at head 83e5935206c0777a5d5ba2a4e67e7c53ec27ebc5 against base 85b76c63b5c9b95ddd1c6bc5c671d5172c2df911. Local, remote, PR, and reviewed heads match. Incoming base changes do not touch the five changed paths. No open merge freeze or failing current-head GitHub check was found. Pending unrelated checks follow the repository’s normal landing policy.

Changelog

Fixed a crash when releasing browser views.

@austinywang austinywang added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 83e5935: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr27166 -f ref=feat-cmux-next-nightly-sigabrt-view-teardown -f platform=macos -f backend_mode=local

web and size checks: green

@blacksmith-sh

blacksmith-sh Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp83cf3zha/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpohxfjjuz/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Bead cx-3k4hx1 created for this PR by the cmux coordinator.

@cursor

cursor Bot commented Oct 29, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (review_browser_retirement): SHIP at 0a86eef

Independent correctness review found no remaining defects in the final diff. The review checked synchronous main-thread retirement, retained off-main cleanup, page/chrome teardown order, ownership transfer, and probe cancellation. Earlier findings (macOS 14-compatible Mutex and probe module visibility/import) were fixed. Native and live Chromium proof are tracked in the PR description.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (review_browser_retirement): SHIP at 05b0fed

Independent exact-head review: no remaining correctness findings. The final test-only repair moves the off-main thread assertion into the synchronous release operation, preserving coverage and application behavior. Earlier macOS 14 Mutex and probe module integration findings are resolved.

@cursor

cursor Bot commented Oct 29, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (review_browser_retirement): SHIP at 83e5935

Independent exact-head correctness review found no remaining defects in the complete change, including the copyable Sendable test owner and the probe autorelease-pool boundary. Native and strict runtime acceptance are still pending; neither compilation nor the earlier partial probe flags is represented as a full pass.

@austinywang austinywang added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 29, 2026
@austinywang
austinywang merged commit 0072601 into feat-cmux-next Oct 29, 2026
228 of 233 checks passed
@austinywang
austinywang deleted the feat-cmux-next-nightly-sigabrt-view-teardown branch October 29, 2026 03:21
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

@teamleaderleo

Copy link
Copy Markdown
Collaborator

For awareness: on feat-cmux-next, BrowserEntryRetirementTests.swift:43 no longer compiles (returning main actor-isolated $0.some.chrome.some as a sending result), so no suite in CmuxNextAppTests runs. #27301 moves the chrome into the window before the entry goes into its Mutex. The test is otherwise the same, and it passes on a fleet Mac.

lawrencecchen added a commit that referenced this pull request Oct 29, 2026
…ncurrency

feat-cmux-next's CmuxNext test build failed since #27166 (compile job 04bdd1343315cf9eea120801):
BrowserEntryRetirementTests.swift:43 returned the main-actor chrome out of the Mutex's nonisolated
withLock closure ("returning main actor-isolated ... as a 'sending' result"). The test now reads the chrome
on main before the entry goes into the Mutex, in a scope that leaves the owner the only reference.
lawrencecchen added a commit that referenced this pull request Oct 29, 2026
…ncurrency

feat-cmux-next's CmuxNext test build failed since #27166 (compile job 04bdd1343315cf9eea120801):
BrowserEntryRetirementTests.swift:43 returned the main-actor chrome out of the Mutex's nonisolated
withLock closure ("returning main actor-isolated ... as a 'sending' result"). The test now reads the chrome
on main before the entry goes into the Mutex, in a scope that leaves the owner the only reference.

Compiled: a2570c53c9143f9d38b3ec0f
teamleaderleo pushed a commit that referenced this pull request Oct 29, 2026
…ncurrency

feat-cmux-next's CmuxNext test build failed since #27166 (compile job 04bdd1343315cf9eea120801):
BrowserEntryRetirementTests.swift:43 returned the main-actor chrome out of the Mutex's nonisolated
withLock closure ("returning main actor-isolated ... as a 'sending' result"). The test now reads the chrome
on main before the entry goes into the Mutex, in a scope that leaves the owner the only reference.

Compiled: a2570c53c9143f9d38b3ec0f
Agent-Session: https://cmux.com/deeplink/agent?id=aid-729c3133f699708e5f40

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 83e59352 Deployed Oct 29, 2026 by austinywang via namespace autofix #5885
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants