Skip to content

fix(macos): order windows out on exit to prevent KVO crash - #820

Merged
lklimek merged 2 commits into
v1.0-devfrom
fix/macos-touchbar-crash
Apr 16, 2026
Merged

lklimek merged 2 commits into
v1.0-devfrom
fix/macos-touchbar-crash

Conversation

@lklimek

@lklimek lklimek commented Apr 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fixes a SIGILL crash on macOS during shutdown caused by _NSTouchBarFinderObservation KVO observers on inconsistent objects
  • Calls [window orderOut:nil] on all windows as the first action in on_exit(), giving AppKit a chance to properly tear down display-related observations while windows and views are still alive
  • No-op on non-macOS platforms

Root cause

During shutdown, winit's event handler teardown triggers a display cycle flush (CATransaction commit). This causes _NSTouchBarFinderObservation invalidate to remove KVO observers from objects that are already in an inconsistent state, throwing an NSException that AppKit catches and deliberately crashes on via _crashOnException:.

Fix

By ordering all windows out before returning from on_exit(), AppKit processes the window removal while everything is still alive and consistent — the KVO observers are properly detached during this orderly removal rather than during the chaotic winit teardown.

Test plan

  • Launch on macOS, close the app window — no crash on exit
  • Launch on macOS, verify the app still shuts down cleanly (background tasks stop, no hang)
  • Build on Linux — compiles without warnings (no-op path)

🤖 Co-authored by Claudius the Magnificent AI Agent

Summary by CodeRabbit

  • Bug Fixes
    • Improved application shutdown on macOS to ensure all windows are properly ordered out and handled before the app completes its exit sequence.

@coderabbitai

coderabbitai Bot commented Apr 9, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8f5f4d7c-99c5-43ff-90e7-1edd5521ec2e

📥 Commits

Reviewing files that changed from the base of the PR and between 594c556 and ee1ed56.

📒 Files selected for processing (3)
  • src/app.rs
  • src/platform/macos.rs
  • src/platform/mod.rs

📝 Walkthrough

Walkthrough

Added a new platform abstraction feature that orders out (hides) all application windows on macOS before application shutdown. The functionality is triggered during app exit via a cross-platform interface that delegates to platform-specific implementations.

Changes

Cohort / File(s) Summary
Window Ordering on Exit
src/app.rs, src/platform/mod.rs, src/platform/macos.rs
Introduced platform abstraction order_out_all_windows() called during AppState::on_exit(). On macOS, enumerates all NSApplication windows and sends orderOut: to each; on other platforms, the function is a no-op. Includes debug logging of window count.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Windows orderOut in a macOS cascade,
As the app makes its graceful fade,
Platform abstractions keep it clean and neat,
Before shutdown's curtain call—complete! 🪟✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: ordering windows out on macOS to prevent a KVO crash during shutdown, which is the primary objective of this pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/macos-touchbar-crash

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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 and usage tips.

@lklimek lklimek linked an issue Apr 9, 2026 that may be closed by this pull request
On macOS, winit's event handler teardown triggers a display cycle flush
that finds _NSTouchBarFinderObservation KVO observers on objects in an
inconsistent state, causing a SIGILL crash via _crashOnException:.

Fix: call [window orderOut:nil] on all windows as the first action in
on_exit(), before returning control to eframe/winit. This lets AppKit
properly tear down its display-related observations while the windows
and their views are still alive.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@lklimek
lklimek force-pushed the fix/macos-touchbar-crash branch from ebf5e94 to 2a97da2 Compare April 9, 2026 08:37
@lklimek lklimek changed the title fix(macos): disable TouchBar to prevent KVO crash on shutdown fix(macos): order windows out on exit to prevent KVO crash Apr 9, 2026
@lklimek
lklimek marked this pull request as ready for review April 16, 2026 11:30
@thepastaclaw

thepastaclaw commented Apr 16, 2026 •

Copy link
Copy Markdown
Collaborator

Review Gate

Commit: ee1ed566

  • ⏳ Debounce: 13m ago (need 30m)

  • ✅ CI checks: builds passed, 0/0 tests passed

  • ✅ CodeRabbit review: comment found

  • ✅ Off-peak hours: off-peak (04:44 AM PT Thursday)

  • Run review now (check to override)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a macOS-specific shutdown crash by hiding (ordering out) all AppKit windows before winit/eframe teardown, allowing AppKit to detach TouchBar-related KVO observers while the window/view graph is still valid.

Changes:

  • Add platform::order_out_all_windows() as a cross-platform (macOS-only implementation) shutdown helper.
  • Implement macOS window enumeration + [window orderOut:nil] via objc2/msg_send!.
  • Invoke the new helper at the start of AppState::on_exit().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/platform/mod.rs Adds a public, macOS-gated order_out_all_windows() entry point.
src/platform/macos.rs Implements ordering out all NSApplication windows using raw AppKit selectors.
src/app.rs Calls order_out_all_windows() first in on_exit() to mitigate macOS teardown crash.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/platform/macos.rs
@lklimek
lklimek merged commit eb7c162 into v1.0-dev Apr 16, 2026
9 checks passed
@lklimek
lklimek deleted the fix/macos-touchbar-crash branch April 16, 2026 11:50
feigeCode added a commit to feigeCode/zed that referenced this pull request Sep 27, 2026
…the process

On a Mac with a Touch Bar, `_NSTouchBarFinder` observes `nextResponder` on every
responder in the active window's chain — the window, its views, and the transient
views AppKit puts there while editing — and retracts those observations later,
from its own display-cycle block. When a retraction is not the first one for the
same observation, Foundation raises `NSRangeException`, AppKit catches it and
calls `-[NSApplication _crashOnException:]`, whose entire purpose is to abort:
the process dies with SIGILL, with no Rust frame on the stack and nothing for us
to catch.

That abort is not caused by anything we release. Retiring the native window
instead of releasing it, and a grace period before the release, were both shipped
and both still aborted; the same exception is reported by apps that share neither
GPUI nor our window lifetime, and one of them root-caused it to queued
invalidations retracting an observation that was already gone:

- longbridge/gpui-kit#3192 — intermittent, on switching the frontmost app
- kodezine/RustyCAN#95 — root cause: queued invalidations, already-removed observer
- dashpay/dash-evo-tool#820 — mitigated by ordering windows out before exit
- johnlindquist/kit#1550 — Electron, reported on the window object
- emilk/egui#2768 — eframe, on quitting the app
- feigeCode/navop#268 and feigeCode/navop#308 — the reports this exists for

`touch_bar_guard` takes over `-[NSApplication _crashOnException:]` and swallows
exactly one exception: an `NSRangeException` whose reason names
`_NSTouchBarFinderObservation`. Nothing is left to clean up in that case — the
retraction it failed on had already happened, so the state AppKit wanted is the
state it has. Every other exception is forwarded to the original implementation,
which aborts exactly as AppKit intended.

Release Notes:

- Fixed a crash on Macs with a Touch Bar, where AppKit aborted the process for an
  exception its own Touch Bar finder raised while retracting its observations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DET crash on closure

3 participants