Tags: zed-industries/zed
Tags
gpui_macos: Release the accessibility adapter when a window is dropped ( #64143) ## Objective Closing a GPUI window on macOS does not release that window's native view tree, so every open/close cycle leaves a `GPUIView`, its `CAMetalLayer` and the renderer's command queue alive in the process. A long-running app that opens and closes windows grows without bound; this is the mechanism described in #50737. The reason is a retain cycle: ``` MacWindowState -> accesskit_adapter (crates/gpui_macos/src/window.rs:710) -> NSWindow.contentView (strongly retained by accesskit_macos) -> GPUIView (a subview of that content view) -> Arc<Mutex<MacWindowState>> in its ivar (back to the start) ``` `accesskit_macos::SubclassingAdapter::for_window` keeps a strong `Id<NSView>` of the window's content view, and both the `NSWindow` and the `GPUIView` store an `Arc<Mutex<MacWindowState>>` in their `windowState` ivar. `MacWindow::drop` resets the delegate and tears down the frame source and the visibility callback, but nothing drops the adapter, so the native view tree outlives the window. Worth noting while reading the existing teardown: `gpui_apple::metal_renderer::destroy()` is currently a no-op (`crates/gpui_apple/src/metal_renderer.rs`), so the `renderer.destroy()` call that is already there is not evidence that GPU-side resources were released — what matters is whether the state that owns the renderer can run its destructor at all. ## Solution One line in `MacWindow::drop`: ```rust let mut this = self.0.lock(); drop(this.accesskit_adapter.take()); this.renderer.destroy(); ``` Nothing else in the teardown changes: - `frame_source.take()` stays. `WindowFrameSource`'s `Drop` cancels `frame_requests`, and that dispatch source's context points at the native view, so it has to be cancelled while the view is still valid. - `visibility_callback.take()` stays, as do the sheet handling and the delegate reset. - No view or layer is released by hand. The native view is already handed to its parent view via `autorelease`; the problem is the cycle, not the ownership. ## Testing Done locally on macOS (aarch64), with `stable`: - `cargo fmt -p gpui_macos -- --check` — clean. - `cargo test -p gpui_macos --lib` — 7 passed, 0 failed. - `cargo clippy -p gpui_macos --release --all-targets --all-features -- --deny warnings` — clean. Note: the adapter is only created when Zed runs with `ZED_EXPERIMENTAL_A11Y=1`, so the leak only affects that path and the fix is a no-op otherwise. What isn't covered: - **No automated test.** The cycle needs a real `NSWindow`/`contentView` pair, and the change is a `Drop` impl; a unit test here would be testing `NSView` ownership rather than this crate's logic. `gpui_macos` has no window-lifecycle test harness today, and adding one is more than this fix should carry. - **No memory measurement by me.** I did not run Instruments/`Allocations` against this change, and the minimal-repro numbers quoted in #50737 were measured on a different fork revision, not this one. So the leak here is established from the ownership graph above rather than from a footprint curve I produced. The follow-up that would settle it is a before/after count of live `GPUIView` / `CAMetalLayer` / command-queue objects after closing a window, with accessibility enabled and disabled; I have not run that, and I am not claiming this explains every memory-growth report. - Only the macOS backend is touched, so there is nothing Windows/Linux-specific here. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards (UI/UX and icon guidelines) — no UI change - [ ] Tests cover the new/changed behavior — not applicable, see "What isn't covered" - [x] Performance impact has been considered and is acceptable ## AI disclosure Built with substantial LLM assistance: I had help reading the `accesskit_macos` and `gpui_macos` ownership graph and drafting this description, and I reviewed every line of the diff myself. The change is one line plus a comment. English isn't my first language, so the prose here was machine-polished; happy to restate anything in plain terms. --- Release Notes: - [GPUI] Fixed a memory leak on macOS where closing a window with accessibility enabled left its native view, `CAMetalLayer` and the renderer's resources alive. - Fixed a memory leak on macOS where closing a window left its native view, `CAMetalLayer` and the renderer's resources alive.
proto: Bump to v0.3.4 (#64372) This PR bumps the version of the Proto extension to v0.3.4. Release Notes: - N/A Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com> Co-authored-by: Finn Evers <finn@zed.dev>
html: Bump to v0.3.2 (#64371) This PR bumps the version of the HTML extension to v0.3.2. Release Notes: - N/A Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com> Co-authored-by: Finn Evers <finn@zed.dev>
glsl: Bump to v0.2.5 (#64370) This PR bumps the version of the GLSL extension to v0.2.5. Release Notes: - N/A Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com> Co-authored-by: Finn Evers <finn@zed.dev>
v1.21.x preview for @JosephTLyons
Bump to 1.20.1 for @ConradIrwin
v1.20.x preview for @dinocosta
PreviousNext