From b0e1dd6b8fddc3049a66c80bb3a63f74886473ba Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:18:04 +0100 Subject: [PATCH 1/6] fix: prevent recursive macOS occlusion callbacks --- apps/desktop-gpui/src/platform.rs | 46 +-- .../src/platform/macos_occlusion.rs | 342 ++++++++++++++++++ 2 files changed, 346 insertions(+), 42 deletions(-) create mode 100644 apps/desktop-gpui/src/platform/macos_occlusion.rs diff --git a/apps/desktop-gpui/src/platform.rs b/apps/desktop-gpui/src/platform.rs index b8899ba5fc..ad226139d6 100644 --- a/apps/desktop-gpui/src/platform.rs +++ b/apps/desktop-gpui/src/platform.rs @@ -82,6 +82,9 @@ pub struct PanelBehavior { #[cfg(target_os = "macos")] pub use mac::*; +#[cfg(target_os = "macos")] +mod macos_occlusion; + #[cfg(target_os = "macos")] mod mac { use gpui::Window; @@ -291,48 +294,7 @@ mod mac { /// (self-delegating) occlusion handler for windows that opened before the /// state ever changed. pub fn install_occlusion_shim() { - use objc2::ffi::{class_addMethod, objc_msgSendSuper, objc_super}; - - unsafe extern "C" fn occlusion_state_shim(this: *mut AnyObject, sel: Sel) -> usize { - unsafe { - let class = (*this).class(); - let Some(superclass) = class.superclass() else { - return 0; - }; - let mut sup = objc_super { - receiver: this.cast(), - super_class: (superclass as *const objc2::runtime::AnyClass).cast(), - }; - let send: unsafe extern "C" fn(*mut objc_super, Sel) -> usize = - std::mem::transmute(objc_msgSendSuper as unsafe extern "C" fn()); - let raw = send(&mut sup, sel); - if raw != 0 { raw | 0x2 } else { raw } - } - } - - for name in ["GPUIWindow", "GPUIPanel"] { - let Some(class) = objc2::runtime::AnyClass::get(name) else { - // The class registers lazily with the first window; the caller - // runs before that only if nothing was opened -- harmless, the - // second call from `kick_display_link` retries. - continue; - }; - let added = unsafe { - class_addMethod( - (class as *const objc2::runtime::AnyClass as *mut objc2::ffi::objc_class) - .cast(), - objc2::sel!(occlusionState).as_ptr(), - Some(std::mem::transmute::< - unsafe extern "C" fn(*mut AnyObject, Sel) -> usize, - unsafe extern "C" fn(), - >(occlusion_state_shim)), - c"Q@:".as_ptr(), - ) - }; - if objc2::runtime::Bool::from_raw(added).as_bool() { - tracing::info!("installed macOS 26 occlusion shim on {name}"); - } - } + super::macos_occlusion::install(); // Same per-class, retried-from-the-same-spots lifecycle, so it rides // along here. diff --git a/apps/desktop-gpui/src/platform/macos_occlusion.rs b/apps/desktop-gpui/src/platform/macos_occlusion.rs new file mode 100644 index 0000000000..9b24d1493c --- /dev/null +++ b/apps/desktop-gpui/src/platform/macos_occlusion.rs @@ -0,0 +1,342 @@ +use std::sync::OnceLock; + +use objc2::{ + ffi::class_addMethod, + runtime::{AnyClass, AnyObject, Bool, Imp, Sel}, + sel, +}; + +type OcclusionStateFn = unsafe extern "C" fn(*mut AnyObject, Sel) -> usize; + +struct OcclusionShim { + original: OnceLock, +} + +impl OcclusionShim { + const fn new() -> Self { + Self { + original: OnceLock::new(), + } + } + + fn install(&self, class: &AnyClass, shim: OcclusionStateFn) -> Result { + if self.original.get().is_some() { + return Ok(false); + } + + let selector = sel!(occlusionState); + class + .verify_sel::<(), usize>(selector) + .map_err(|error| error.to_string())?; + let method = class + .instance_method(selector) + .ok_or_else(|| "occlusionState implementation is missing".to_string())?; + let original = + unsafe { std::mem::transmute::(method.implementation()) }; + if std::ptr::fn_addr_eq(original, shim) { + return Err("occlusionState already points to the replacement".to_string()); + } + + // Save the original before publishing the override. Looking up a superclass + // from the receiver can re-enter this shim through an AppKit dynamic subclass. + if self.original.set(original).is_err() { + return Ok(false); + } + let added = unsafe { + class_addMethod( + (class as *const AnyClass).cast_mut().cast(), + selector.as_ptr(), + Some(std::mem::transmute::< + OcclusionStateFn, + unsafe extern "C" fn(), + >(shim)), + c"Q@:".as_ptr(), + ) + }; + Ok(Bool::from_raw(added).as_bool()) + } + + unsafe fn state(&self, receiver: *mut AnyObject, selector: Sel) -> usize { + let Some(original) = self.original.get() else { + return 0; + }; + let raw = unsafe { original(receiver, selector) }; + if raw != 0 { raw | 0x2 } else { raw } + } +} + +static WINDOW_SHIM: OcclusionShim = OcclusionShim::new(); +static PANEL_SHIM: OcclusionShim = OcclusionShim::new(); + +unsafe extern "C" fn window_occlusion_state(receiver: *mut AnyObject, selector: Sel) -> usize { + unsafe { WINDOW_SHIM.state(receiver, selector) } +} + +unsafe extern "C" fn panel_occlusion_state(receiver: *mut AnyObject, selector: Sel) -> usize { + unsafe { PANEL_SHIM.state(receiver, selector) } +} + +pub(super) fn install() { + for (name, state, implementation) in [ + ( + "GPUIWindow", + &WINDOW_SHIM, + window_occlusion_state as OcclusionStateFn, + ), + ( + "GPUIPanel", + &PANEL_SHIM, + panel_occlusion_state as OcclusionStateFn, + ), + ] { + let Some(class) = AnyClass::get(name) else { + continue; + }; + match state.install(class, implementation) { + Ok(true) => tracing::info!("installed macOS occlusion shim on {name}"), + Ok(false) => {} + Err(error) => tracing::warn!(%error, name, "could not install macOS occlusion shim"), + } + } +} + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use objc2::{ + ClassType, msg_send, msg_send_id, + rc::Id, + runtime::{ClassBuilder, NSObject}, + }; + + use super::*; + + thread_local! { + static RAW_STATE: Cell = const { Cell::new(0) }; + static WINDOW_CALLS: Cell = const { Cell::new(0) }; + static PANEL_CALLS: Cell = const { Cell::new(0) }; + } + + struct Classes { + window: &'static AnyClass, + panel: &'static AnyClass, + inherited_window: &'static AnyClass, + inherited_panel: &'static AnyClass, + overridden_window: &'static AnyClass, + overridden_panel: &'static AnyClass, + } + + unsafe extern "C" fn native_window_state(_receiver: *mut AnyObject, _selector: Sel) -> usize { + WINDOW_CALLS.set(WINDOW_CALLS.get() + 1); + RAW_STATE.get() + } + + unsafe extern "C" fn native_panel_state(_receiver: *mut AnyObject, _selector: Sel) -> usize { + PANEL_CALLS.set(PANEL_CALLS.get() + 1); + RAW_STATE.get() + } + + unsafe extern "C" fn overriding_window_state( + receiver: *mut AnyObject, + _selector: Sel, + ) -> usize { + unsafe { msg_send![super(receiver, classes().window), occlusionState] } + } + + unsafe extern "C" fn overriding_panel_state(receiver: *mut AnyObject, _selector: Sel) -> usize { + unsafe { msg_send![super(receiver, classes().panel), occlusionState] } + } + + fn inherit(name: &str, superclass: &AnyClass) -> &'static AnyClass { + ClassBuilder::new(name, superclass).unwrap().register() + } + + fn with_method( + name: &str, + superclass: &AnyClass, + implementation: OcclusionStateFn, + ) -> &'static AnyClass { + let mut builder = ClassBuilder::new(name, superclass).unwrap(); + unsafe { builder.add_method(sel!(occlusionState), implementation) }; + builder.register() + } + + fn classes() -> &'static Classes { + static CLASSES: OnceLock = OnceLock::new(); + CLASSES.get_or_init(|| { + let native_window = with_method( + "CapOcclusionTestNativeWindow", + NSObject::class(), + native_window_state, + ); + let native_panel = with_method( + "CapOcclusionTestNativePanel", + native_window, + native_panel_state, + ); + let window = inherit("CapOcclusionTestWindow", native_window); + let panel = inherit("CapOcclusionTestPanel", native_panel); + assert!(WINDOW_SHIM.install(window, window_occlusion_state).unwrap()); + assert!(PANEL_SHIM.install(panel, panel_occlusion_state).unwrap()); + + let mut inherited_window = window; + let mut inherited_panel = panel; + for depth in 0..4 { + inherited_window = inherit( + &format!("CapOcclusionTestWindowSubclass{depth}"), + inherited_window, + ); + inherited_panel = inherit( + &format!("CapOcclusionTestPanelSubclass{depth}"), + inherited_panel, + ); + } + let window_override = with_method( + "CapOcclusionTestWindowOverride", + window, + overriding_window_state, + ); + let panel_override = with_method( + "CapOcclusionTestPanelOverride", + panel, + overriding_panel_state, + ); + Classes { + window, + panel, + inherited_window, + inherited_panel, + overridden_window: inherit( + "CapOcclusionTestInheritedWindowOverride", + window_override, + ), + overridden_panel: inherit("CapOcclusionTestInheritedPanelOverride", panel_override), + } + }) + } + + fn verify_states(class: &AnyClass, expected_window_calls: usize, expected_panel_calls: usize) { + let instance: Id = unsafe { msg_send_id![class, new] }; + for raw in [0, 0x2, 0x2000, 0x2002] { + RAW_STATE.set(raw); + WINDOW_CALLS.set(0); + PANEL_CALLS.set(0); + let state: usize = unsafe { msg_send![&*instance, occlusionState] }; + assert_eq!(state, if raw != 0 { raw | 0x2 } else { raw }); + assert_eq!(WINDOW_CALLS.get(), expected_window_calls); + assert_eq!(PANEL_CALLS.get(), expected_panel_calls); + } + } + + #[test] + fn windows_and_panels_keep_their_own_native_implementation() { + verify_states(classes().window, 1, 0); + verify_states(classes().panel, 0, 1); + } + + #[test] + fn inherited_shims_do_not_recurse() { + verify_states(classes().inherited_window, 1, 0); + verify_states(classes().inherited_panel, 0, 1); + } + + #[test] + fn inherited_overrides_can_call_super_without_reentering_themselves() { + verify_states(classes().overridden_window, 1, 0); + verify_states(classes().overridden_panel, 0, 1); + } + + #[test] + fn repeated_installation_keeps_the_original_implementation() { + let classes = classes(); + for _ in 0..10 { + assert!( + !WINDOW_SHIM + .install(classes.window, window_occlusion_state) + .unwrap() + ); + assert!( + !PANEL_SHIM + .install(classes.panel, panel_occlusion_state) + .unwrap() + ); + } + verify_states(classes.overridden_window, 1, 0); + verify_states(classes.overridden_panel, 0, 1); + } + + #[test] + fn promoting_a_window_switches_to_the_panel_implementation() { + let classes = classes(); + let instance: Id = unsafe { msg_send_id![classes.window, new] }; + RAW_STATE.set(0x2000); + WINDOW_CALLS.set(0); + PANEL_CALLS.set(0); + let before: usize = unsafe { msg_send![&*instance, occlusionState] }; + assert_eq!(before, 0x2002); + + assert_eq!( + classes.window.instance_size(), + classes.panel.instance_size() + ); + unsafe { AnyObject::set_class(&instance, classes.panel) }; + let after: usize = unsafe { msg_send![&*instance, occlusionState] }; + assert_eq!(after, 0x2002); + assert_eq!(WINDOW_CALLS.get(), 1); + assert_eq!(PANEL_CALLS.get(), 1); + } + + #[test] + fn incompatible_native_method_does_not_install_a_shim() { + unsafe extern "C" fn wrong_signature(_receiver: *mut AnyObject, _selector: Sel) -> f64 { + 0.0 + } + + let state = OcclusionShim::new(); + let mut builder = + ClassBuilder::new("CapOcclusionTestWrongSignature", NSObject::class()).unwrap(); + unsafe { + builder.add_method( + sel!(occlusionState), + wrong_signature as unsafe extern "C" fn(_, _) -> _, + ); + } + let class = builder.register(); + assert!(state.install(class, window_occlusion_state).is_err()); + assert!(state.original.get().is_none()); + } + + #[test] + fn existing_class_override_is_not_replaced() { + let state = OcclusionShim::new(); + let class = with_method( + "CapOcclusionTestExistingOverride", + NSObject::class(), + native_window_state, + ); + assert!(!state.install(class, window_occlusion_state).unwrap()); + let instance: Id = unsafe { msg_send_id![class, new] }; + RAW_STATE.set(0x2000); + let raw: usize = unsafe { msg_send![&*instance, occlusionState] }; + assert_eq!(raw, 0x2000); + } + + #[test] + fn missing_native_method_does_not_install_a_shim() { + let state = OcclusionShim::new(); + let class = inherit("CapOcclusionTestMissingMethod", NSObject::class()); + assert!(state.install(class, window_occlusion_state).is_err()); + assert!(state.original.get().is_none()); + assert!(class.instance_method(sel!(occlusionState)).is_none()); + } + + #[test] + fn an_uninitialized_shim_returns_no_visibility() { + let state = OcclusionShim::new(); + assert_eq!( + unsafe { state.state(std::ptr::null_mut(), sel!(occlusionState)) }, + 0 + ); + } +} From bc46c567f0227be4e44f3508a202223c69757eed Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:18:04 +0100 Subject: [PATCH 2/6] fix: handle malformed macOS camera device IDs --- crates/camera/src/macos.rs | 42 ++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/crates/camera/src/macos.rs b/crates/camera/src/macos.rs index 722823e2ef..fece38486f 100644 --- a/crates/camera/src/macos.rs +++ b/crates/camera/src/macos.rs @@ -62,18 +62,22 @@ impl CameraInfo { impl ModelID { fn from_avfoundation(device: &cidre::av::capture::Device) -> Option { let unique_id = device.unique_id().to_string(); - if unique_id.len() < 8 { - return None; - } + Self::from_avfoundation_unique_id(&unique_id) + } - let vid = unique_id[unique_id.len() - 2 * 4..unique_id.len() - 4].to_string(); - let pid = unique_id[unique_id.len() - 4..].to_string(); + fn from_avfoundation_unique_id(unique_id: &str) -> Option { + let suffix = unique_id.get(unique_id.len().checked_sub(8)?..)?; + let vid = suffix.get(..4)?; + let pid = suffix.get(4..)?; if vid == "0000" && pid == "0001" { return None; } - Some(Self { vid, pid }) + Some(Self { + vid: vid.to_string(), + pid: pid.to_string(), + }) } } @@ -372,3 +376,29 @@ impl NativeCapturedFrame { &self.0 } } + +#[cfg(test)] +mod model_id_tests { + use super::ModelID; + + #[test] + fn usb_camera_ids_preserve_the_vendor_and_product_suffix() { + for unique_id in ["0x12340000046d082d", "046d082d", "カメラ046d082d"] { + let model = ModelID::from_avfoundation_unique_id(unique_id).unwrap(); + assert_eq!(model.vid, "046d"); + assert_eq!(model.pid, "082d"); + } + } + + #[test] + fn malformed_camera_ids_fall_back_to_the_device_id() { + for unique_id in ["", "short", "cameraé123", "é1234567", "123é456"] { + assert!(ModelID::from_avfoundation_unique_id(unique_id).is_none()); + } + } + + #[test] + fn builtin_camera_ids_still_fall_back_to_the_device_id() { + assert!(ModelID::from_avfoundation_unique_id("0x1234000000000001").is_none()); + } +} From 3aff505f821221513997bb274d9f35440e8a9e44 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:18:04 +0100 Subject: [PATCH 3/6] fix: retain Metal surfaces and report renderer initialization errors --- apps/desktop-gpui/patches/zed-gpui.patch | 586 +++++++++++++++++++---- apps/desktop-gpui/src/main.rs | 6 +- 2 files changed, 504 insertions(+), 88 deletions(-) diff --git a/apps/desktop-gpui/patches/zed-gpui.patch b/apps/desktop-gpui/patches/zed-gpui.patch index eefbff8935..d8f31f8169 100644 --- a/apps/desktop-gpui/patches/zed-gpui.patch +++ b/apps/desktop-gpui/patches/zed-gpui.patch @@ -66,7 +66,7 @@ index 0480dc9574..484179d778 100644 + corner_radii, + source_uv, diff --git a/crates/gpui_macos/src/metal_renderer.rs b/crates/gpui_macos/src/metal_renderer.rs -index c93a383c38..11506dbb62 100644 +index c93a383c38..1d35af12f6 100644 --- a/crates/gpui_macos/src/metal_renderer.rs +++ b/crates/gpui_macos/src/metal_renderer.rs @@ -10,3 +10,3 @@ use gpui::{ @@ -79,12 +79,89 @@ index c93a383c38..11506dbb62 100644 @@ -19,2 +19,3 @@ use core_video::{ - metal_texture::CVMetalTextureGetTexture, metal_texture_cache::CVMetalTextureCache, - pixel_buffer::kCVPixelFormatType_420YpCbCr8BiPlanarFullRange, -+ metal_texture::CVMetalTextureGetTexture, ++ metal_texture::{CVMetalTexture, CVMetalTextureGetTexture}, + metal_texture_cache::CVMetalTextureCache, + pixel_buffer::{kCVPixelFormatType_32BGRA, kCVPixelFormatType_420YpCbCr8BiPlanarFullRange}, -@@ -178,0 +180 @@ pub(crate) struct MetalRenderer { +@@ -50,0 +52 @@ const SCRATCH_RELEASE_AFTER_FRAMES: u32 = 30; ++const SURFACE_ERROR_LOG_INTERVAL: std::time::Duration = std::time::Duration::from_secs(5); +@@ -55,7 +57 @@ pub(crate) type Renderer = MetalRenderer; +-pub(crate) unsafe fn new_renderer( +- context: self::Context, +- _native_window: *mut c_void, +- _native_view: *mut c_void, +- _bounds: gpui::Size, +- transparent: bool, +-) -> Renderer { ++pub(crate) fn new_renderer(context: self::Context, transparent: bool) -> Result { +@@ -173,0 +170 @@ pub(crate) struct MetalRenderer { ++ surface_error_last_log: Option, +@@ -178,0 +176 @@ pub(crate) struct MetalRenderer { + bgra_surfaces_pipeline_state: metal::RenderPipelineState, -@@ -393,0 +396,8 @@ impl MetalRenderer { +@@ -211,2 +209,5 @@ impl MetalRenderer { +- pub fn new(instance_buffer_pool: Arc>, transparent: bool) -> Self { +- let device = Self::create_device(); ++ pub fn new( ++ instance_buffer_pool: Arc>, ++ transparent: bool, ++ ) -> Result { ++ let device = Self::create_device()?; +@@ -242,2 +243,2 @@ impl MetalRenderer { +- pub fn new_headless(instance_buffer_pool: Arc>) -> Self { +- let device = Self::create_device(); ++ pub fn new_headless(instance_buffer_pool: Arc>) -> Result { ++ let device = Self::create_device()?; +@@ -247 +248 @@ impl MetalRenderer { +- fn create_device() -> metal::Device { ++ fn create_device() -> Result { +@@ -255 +256 @@ impl MetalRenderer { +- d ++ Ok(d) +@@ -262,4 +263,2 @@ impl MetalRenderer { +- metal::Device::system_default().unwrap_or_else(|| { +- log::error!("unable to access a compatible graphics device"); +- std::process::exit(1); +- }) ++ metal::Device::system_default() ++ .ok_or_else(|| anyhow::anyhow!("unable to access a compatible graphics device")) +@@ -274,9 +273,2 @@ impl MetalRenderer { +- ) -> Self { +- #[cfg(feature = "runtime_shaders")] +- let library = device +- .new_library_with_source(&SHADERS_SOURCE_FILE, &metal::CompileOptions::new()) +- .expect("error building metal library"); +- #[cfg(not(feature = "runtime_shaders"))] +- let library = device +- .new_library_with_data(SHADERS_METALLIB) +- .expect("error building metal library"); ++ ) -> Result { ++ let library = build_shader_library(&device)?; +@@ -327 +319 @@ impl MetalRenderer { +- ); ++ )?; +@@ -335 +327 @@ impl MetalRenderer { +- ); ++ )?; +@@ -343 +335 @@ impl MetalRenderer { +- ); ++ )?; +@@ -353 +345 @@ impl MetalRenderer { +- ); ++ )?; +@@ -361 +353 @@ impl MetalRenderer { +- ); ++ )?; +@@ -369 +361 @@ impl MetalRenderer { +- ); ++ )?; +@@ -377 +369 @@ impl MetalRenderer { +- ); ++ )?; +@@ -385 +377 @@ impl MetalRenderer { +- ); ++ )?; +@@ -393 +385,9 @@ impl MetalRenderer { +- ); ++ )?; + let bgra_surfaces_pipeline_state = build_pipeline_state( + &device, + &library, @@ -92,10 +169,25 @@ index c93a383c38..11506dbb62 100644 + "surface_vertex", + "surface_bgra_fragment", + MTLPixelFormat::BGRA8Unorm, -+ ); -@@ -422,0 +433 @@ impl MetalRenderer { ++ )?; +@@ -397,2 +397,4 @@ impl MetalRenderer { +- let core_video_texture_cache = +- CVMetalTextureCache::new(None, device.clone(), None).unwrap(); ++ let core_video_texture_cache = CVMetalTextureCache::new(None, device.clone(), None) ++ .map_err(|status| { ++ anyhow::anyhow!("failed to create Core Video Metal texture cache: {status}") ++ })?; +@@ -400 +402 @@ impl MetalRenderer { +- Self { ++ Ok(Self { +@@ -417,0 +420 @@ impl MetalRenderer { ++ surface_error_last_log: None, +@@ -422,0 +426 @@ impl MetalRenderer { + bgra_surfaces_pipeline_state, -@@ -489,3 +500,7 @@ impl MetalRenderer { +@@ -432 +436 @@ impl MetalRenderer { +- } ++ }) +@@ -489,3 +493,7 @@ impl MetalRenderer { - if self.path_intermediate_texture.as_ref().is_some_and(|texture| { - texture.width() == size.width.0 as u64 && texture.height() == size.height.0 as u64 - }) { @@ -106,25 +198,34 @@ index c93a383c38..11506dbb62 100644 + texture.width() == size.width.0 as u64 && texture.height() == size.height.0 as u64 + }) + { -@@ -1196,3 +1211 @@ impl MetalRenderer { +@@ -921,0 +930 @@ impl MetalRenderer { ++ let mut retained_surface_textures = CommandBufferResources::default(); +@@ -1141,0 +1151 @@ impl MetalRenderer { ++ &mut retained_surface_textures, +@@ -1172,0 +1183,2 @@ impl MetalRenderer { ++ retained_surface_textures.retain_until_completion(command_buffer); ++ +@@ -1196,3 +1208 @@ impl MetalRenderer { - texture - .as_ref() - .map_or(0, |t| t.width() * t.height() * 4) + texture.as_ref().map_or(0, |t| t.width() * t.height() * 4) -@@ -1385,2 +1398 @@ impl MetalRenderer { +@@ -1385,2 +1395 @@ impl MetalRenderer { - let alloc: *mut objc::runtime::Object = - msg_send![class!(MPSImageGaussianBlur), alloc]; + let alloc: *mut objc::runtime::Object = msg_send![class!(MPSImageGaussianBlur), alloc]; -@@ -1444,2 +1456,4 @@ impl MetalRenderer { +@@ -1444,2 +1453,4 @@ impl MetalRenderer { - command_encoder - .set_fragment_texture(BackdropBlurInputIndex::SourceTexture as u64, Some(source_texture)); + command_encoder.set_fragment_texture( + BackdropBlurInputIndex::SourceTexture as u64, + Some(source_texture), + ); -@@ -1898 +1911,0 @@ impl MetalRenderer { +@@ -1896,0 +1908 @@ impl MetalRenderer { ++ retained_surface_textures: &mut CommandBufferResources, +@@ -1898 +1909,0 @@ impl MetalRenderer { - command_encoder.set_render_pipeline_state(&self.surfaces_pipeline_state); -@@ -1916,27 +1929,63 @@ impl MetalRenderer { +@@ -1916,8 +1927,3 @@ impl MetalRenderer { - assert_eq!( - surface.image_buffer.get_pixel_format(), - kCVPixelFormatType_420YpCbCr8BiPlanarFullRange @@ -133,99 +234,106 @@ index c93a383c38..11506dbb62 100644 - let y_texture = self - .core_video_texture_cache - .create_texture_from_image( -- surface.image_buffer.as_concrete_TypeRef(), -- None, -- MTLPixelFormat::R8Unorm, -- surface.image_buffer.get_width_of_plane(0), -- surface.image_buffer.get_height_of_plane(0), -- 0, ++ let pixel_format = surface.image_buffer.get_pixel_format(); ++ if pixel_format == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange { ++ let y_texture = match self.core_video_texture_cache.create_texture_from_image( +@@ -1930,5 +1936,10 @@ impl MetalRenderer { - ) - .unwrap(); - let cb_cr_texture = self - .core_video_texture_cache - .create_texture_from_image( -- surface.image_buffer.as_concrete_TypeRef(), -- None, -- MTLPixelFormat::RG8Unorm, -- surface.image_buffer.get_width_of_plane(1), -- surface.image_buffer.get_height_of_plane(1), -- 1, ++ ) { ++ Ok(texture) => texture, ++ Err(status) => { ++ self.log_surface_texture_error(format_args!( ++ "failed to create Metal luma texture for surface frame: {status}" ++ )); ++ continue; ++ } ++ }; ++ let cb_cr_texture = match self.core_video_texture_cache.create_texture_from_image( +@@ -1941,2 +1952,67 @@ impl MetalRenderer { - ) - .unwrap(); -+ // CVMetalTexture wrappers must outlive the draw call below; the -+ // command encoder retains the underlying MTLTextures, but the -+ // texture cache may recycle them once the wrapper is released. -+ let pixel_format = surface.image_buffer.get_pixel_format(); -+ let _plane_textures = if pixel_format == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange -+ { ++ ) { ++ Ok(texture) => texture, ++ Err(status) => { ++ self.log_surface_texture_error(format_args!( ++ "failed to create Metal chroma texture for surface frame: {status}" ++ )); ++ continue; ++ } ++ }; ++ let y_metal_texture = ++ unsafe { CVMetalTextureGetTexture(y_texture.as_concrete_TypeRef()) }; ++ let cb_cr_metal_texture = ++ unsafe { CVMetalTextureGetTexture(cb_cr_texture.as_concrete_TypeRef()) }; ++ if y_metal_texture.is_null() || cb_cr_metal_texture.is_null() { ++ self.log_surface_texture_error(format_args!( ++ "Core Video returned a null Metal texture for surface frame" ++ )); ++ continue; ++ } ++ + command_encoder.set_render_pipeline_state(&self.surfaces_pipeline_state); -+ let y_texture = self -+ .core_video_texture_cache -+ .create_texture_from_image( -+ surface.image_buffer.as_concrete_TypeRef(), -+ None, -+ MTLPixelFormat::R8Unorm, -+ surface.image_buffer.get_width_of_plane(0), -+ surface.image_buffer.get_height_of_plane(0), -+ 0, -+ ) -+ .unwrap(); -+ let cb_cr_texture = self -+ .core_video_texture_cache -+ .create_texture_from_image( -+ surface.image_buffer.as_concrete_TypeRef(), -+ None, -+ MTLPixelFormat::RG8Unorm, -+ surface.image_buffer.get_width_of_plane(1), -+ surface.image_buffer.get_height_of_plane(1), -+ 1, -+ ) -+ .unwrap(); + command_encoder.set_fragment_texture(SurfaceInputIndex::YTexture as u64, unsafe { -+ let texture = CVMetalTextureGetTexture(y_texture.as_concrete_TypeRef()); -+ Some(metal::TextureRef::from_ptr(texture as *mut _)) ++ Some(metal::TextureRef::from_ptr(y_metal_texture as *mut _)) + }); -+ command_encoder.set_fragment_texture( -+ SurfaceInputIndex::CbCrTexture as u64, -+ unsafe { -+ let texture = CVMetalTextureGetTexture(cb_cr_texture.as_concrete_TypeRef()); -+ Some(metal::TextureRef::from_ptr(texture as *mut _)) -+ }, -+ ); -+ (y_texture, Some(cb_cr_texture)) ++ command_encoder ++ .set_fragment_texture(SurfaceInputIndex::CbCrTexture as u64, unsafe { ++ Some(metal::TextureRef::from_ptr(cb_cr_metal_texture as *mut _)) ++ }); ++ retained_surface_textures.push(y_texture); ++ retained_surface_textures.push(cb_cr_texture); + } else if pixel_format == kCVPixelFormatType_32BGRA { ++ let color_texture = match self.core_video_texture_cache.create_texture_from_image( ++ surface.image_buffer.as_concrete_TypeRef(), ++ None, ++ MTLPixelFormat::BGRA8Unorm, ++ surface.image_buffer.get_width(), ++ surface.image_buffer.get_height(), ++ 0, ++ ) { ++ Ok(texture) => texture, ++ Err(status) => { ++ self.log_surface_texture_error(format_args!( ++ "failed to create Metal BGRA texture for surface frame: {status}" ++ )); ++ continue; ++ } ++ }; ++ let metal_texture = ++ unsafe { CVMetalTextureGetTexture(color_texture.as_concrete_TypeRef()) }; ++ if metal_texture.is_null() { ++ self.log_surface_texture_error(format_args!( ++ "Core Video returned a null Metal texture for surface frame" ++ )); ++ continue; ++ } ++ + command_encoder.set_render_pipeline_state(&self.bgra_surfaces_pipeline_state); -+ let color_texture = self -+ .core_video_texture_cache -+ .create_texture_from_image( -+ surface.image_buffer.as_concrete_TypeRef(), -+ None, -+ MTLPixelFormat::BGRA8Unorm, -+ surface.image_buffer.get_width(), -+ surface.image_buffer.get_height(), -+ 0, -+ ) -+ .unwrap(); + command_encoder.set_fragment_texture(SurfaceInputIndex::YTexture as u64, unsafe { -+ let texture = CVMetalTextureGetTexture(color_texture.as_concrete_TypeRef()); -+ Some(metal::TextureRef::from_ptr(texture as *mut _)) ++ Some(metal::TextureRef::from_ptr(metal_texture as *mut _)) + }); -+ (color_texture, None) ++ retained_surface_textures.push(color_texture); + } else { -+ log::error!("unsupported surface pixel format: {pixel_format:#x}"); ++ self.log_surface_texture_error(format_args!( ++ "unsupported surface pixel format: {pixel_format:#x}" ++ )); + continue; -+ }; -@@ -1945 +1994 @@ impl MetalRenderer { ++ } +@@ -1945 +2021 @@ impl MetalRenderer { - let next_offset = *instance_offset + mem::size_of::(); + let next_offset = *instance_offset + mem::size_of::(); -@@ -1954,0 +2004,6 @@ impl MetalRenderer { +@@ -1954,0 +2031,6 @@ impl MetalRenderer { + // The fragment reads corner radii from the same instance record. + command_encoder.set_fragment_buffer( + SurfaceInputIndex::Surfaces as u64, + Some(&instance_buffer.metal_buffer), + *instance_offset as u64, + ); -@@ -1960,9 +2014,0 @@ impl MetalRenderer { +@@ -1960,9 +2041,0 @@ impl MetalRenderer { - // let y_texture = y_texture.get_texture().unwrap(). - command_encoder.set_fragment_texture(SurfaceInputIndex::YTexture as u64, unsafe { - let texture = CVMetalTextureGetTexture(y_texture.as_concrete_TypeRef()); @@ -235,16 +343,279 @@ index c93a383c38..11506dbb62 100644 - let texture = CVMetalTextureGetTexture(cb_cr_texture.as_concrete_TypeRef()); - Some(metal::TextureRef::from_ptr(texture as *mut _)) - }); -@@ -1978,0 +2025,2 @@ impl MetalRenderer { +@@ -1978,0 +2052,2 @@ impl MetalRenderer { + corner_radii: surface.corner_radii, + source_uv: surface.source_uv, -@@ -2246 +2294 @@ pub struct PathSprite { +@@ -1987,0 +2063,50 @@ impl MetalRenderer { ++ ++ fn log_surface_texture_error(&mut self, error: std::fmt::Arguments<'_>) { ++ let now = std::time::Instant::now(); ++ if !surface_error_log_due(self.surface_error_last_log, now) { ++ return; ++ } ++ self.surface_error_last_log = Some(now); ++ log::error!("{error}"); ++ } ++} ++ ++struct CommandBufferResources { ++ resources: Vec, ++} ++ ++impl Default for CommandBufferResources { ++ fn default() -> Self { ++ Self { ++ resources: Vec::new(), ++ } ++ } ++} ++ ++impl CommandBufferResources { ++ fn push(&mut self, resource: T) { ++ self.resources.push(resource); ++ } ++} ++ ++impl CommandBufferResources { ++ fn retain_until_completion(self, command_buffer: &metal::CommandBufferRef) { ++ if self.resources.is_empty() { ++ return; ++ } ++ ++ // Command encoders retain MTLTexture, but Core Video may recycle its ++ // cache-managed backing unless the CVMetalTexture wrapper also survives. ++ let resources = Cell::new(Some(self.resources)); ++ let block = ConcreteBlock::new(move |_| { ++ drop(resources.take()); ++ }); ++ let block = block.copy(); ++ command_buffer.add_completed_handler(&block); ++ } ++} ++ ++fn surface_error_log_due(last_log: Option, now: std::time::Instant) -> bool { ++ last_log.is_none_or(|last_log| { ++ now.saturating_duration_since(last_log) >= SURFACE_ERROR_LOG_INTERVAL ++ }) +@@ -2016,0 +2142,15 @@ fn new_command_encoder_for_texture<'a>( ++fn build_shader_library(device: &metal::DeviceRef) -> Result { ++ #[cfg(feature = "runtime_shaders")] ++ { ++ device ++ .new_library_with_source(SHADERS_SOURCE_FILE, &metal::CompileOptions::new()) ++ .map_err(|error| anyhow::anyhow!("failed to build Metal shader library: {error}")) ++ } ++ #[cfg(not(feature = "runtime_shaders"))] ++ { ++ device ++ .new_library_with_data(SHADERS_METALLIB) ++ .map_err(|error| anyhow::anyhow!("failed to load Metal shader library: {error}")) ++ } ++} ++ +@@ -2024 +2164 @@ fn build_pipeline_state( +-) -> metal::RenderPipelineState { ++) -> Result { +@@ -2027 +2167,5 @@ fn build_pipeline_state( +- .expect("error locating vertex function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal vertex function {vertex_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2030 +2174,5 @@ fn build_pipeline_state( +- .expect("error locating fragment function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal fragment function {fragment_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2036 +2184,4 @@ fn build_pipeline_state( +- let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); ++ let color_attachment = descriptor ++ .color_attachments() ++ .object_at(0) ++ .ok_or_else(|| anyhow::anyhow!("Metal pipeline {label} has no color attachment"))?; +@@ -2053 +2204 @@ fn build_pipeline_state( +- .expect("could not create render pipeline state") ++ .map_err(|error| anyhow::anyhow!("failed to create Metal pipeline {label}: {error}")) +@@ -2063 +2214 @@ fn build_pipeline_state_no_blend( +-) -> metal::RenderPipelineState { ++) -> Result { +@@ -2066 +2217,5 @@ fn build_pipeline_state_no_blend( +- .expect("error locating vertex function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal vertex function {vertex_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2069 +2224,5 @@ fn build_pipeline_state_no_blend( +- .expect("error locating fragment function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal fragment function {fragment_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2075 +2234,4 @@ fn build_pipeline_state_no_blend( +- let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); ++ let color_attachment = descriptor ++ .color_attachments() ++ .object_at(0) ++ .ok_or_else(|| anyhow::anyhow!("Metal pipeline {label} has no color attachment"))?; +@@ -2081 +2243 @@ fn build_pipeline_state_no_blend( +- .expect("could not create render pipeline state") ++ .map_err(|error| anyhow::anyhow!("failed to create Metal pipeline {label}: {error}")) +@@ -2110 +2272 @@ fn build_path_sprite_pipeline_state( +-) -> metal::RenderPipelineState { ++) -> Result { +@@ -2113 +2275,5 @@ fn build_path_sprite_pipeline_state( +- .expect("error locating vertex function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal vertex function {vertex_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2116 +2282,5 @@ fn build_path_sprite_pipeline_state( +- .expect("error locating fragment function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal fragment function {fragment_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2122 +2292,4 @@ fn build_path_sprite_pipeline_state( +- let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); ++ let color_attachment = descriptor ++ .color_attachments() ++ .object_at(0) ++ .ok_or_else(|| anyhow::anyhow!("Metal pipeline {label} has no color attachment"))?; +@@ -2136 +2309 @@ fn build_path_sprite_pipeline_state( +- .expect("could not create render pipeline state") ++ .map_err(|error| anyhow::anyhow!("failed to create Metal pipeline {label}: {error}")) +@@ -2147 +2320 @@ fn build_path_rasterization_pipeline_state( +-) -> metal::RenderPipelineState { ++) -> Result { +@@ -2150 +2323,5 @@ fn build_path_rasterization_pipeline_state( +- .expect("error locating vertex function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal vertex function {vertex_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2153 +2330,5 @@ fn build_path_rasterization_pipeline_state( +- .expect("error locating fragment function"); ++ .map_err(|error| { ++ anyhow::anyhow!( ++ "failed to locate Metal fragment function {fragment_fn_name} for {label}: {error}" ++ ) ++ })?; +@@ -2163 +2344,4 @@ fn build_path_rasterization_pipeline_state( +- let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); ++ let color_attachment = descriptor ++ .color_attachments() ++ .object_at(0) ++ .ok_or_else(|| anyhow::anyhow!("Metal pipeline {label} has no color attachment"))?; +@@ -2175 +2359 @@ fn build_path_rasterization_pipeline_state( +- .expect("could not create render pipeline state") ++ .map_err(|error| anyhow::anyhow!("failed to create Metal pipeline {label}: {error}")) +@@ -2246 +2430 @@ pub struct PathSprite { -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, PartialEq)] -@@ -2250,0 +2299,2 @@ pub struct SurfaceBounds { +@@ -2250,0 +2435,2 @@ pub struct SurfaceBounds { + pub corner_radii: Corners, + pub source_uv: Bounds, -@@ -2454 +2504,4 @@ mod backdrop_blur_tests { +@@ -2260 +2446 @@ impl MetalHeadlessRenderer { +- pub fn new() -> Self { ++ pub fn new() -> Result { +@@ -2262,2 +2448,2 @@ impl MetalHeadlessRenderer { +- let renderer = MetalRenderer::new_headless(instance_buffer_pool); +- Self { renderer } ++ let renderer = MetalRenderer::new_headless(instance_buffer_pool)?; ++ Ok(Self { renderer }) +@@ -2285,0 +2472,79 @@ impl gpui::PlatformHeadlessRenderer for MetalHeadlessRenderer { ++#[cfg(test)] ++mod renderer_hardening_tests { ++ use super::*; ++ use std::sync::atomic::{AtomicBool, Ordering}; ++ ++ struct DropProbe(Arc); ++ ++ impl Drop for DropProbe { ++ fn drop(&mut self) { ++ self.0.store(true, Ordering::Release); ++ } ++ } ++ ++ #[test] ++ fn missing_pipeline_function_returns_error() -> Result<()> { ++ let Some(device) = metal::Device::system_default() else { ++ return Ok(()); ++ }; ++ let library = build_shader_library(&device)?; ++ let result = build_pipeline_state( ++ &device, ++ &library, ++ "invalid_test_pipeline", ++ "missing_vertex_function", ++ "quad_fragment", ++ MTLPixelFormat::BGRA8Unorm, ++ ); ++ ++ let Err(error) = result else { ++ anyhow::bail!("pipeline unexpectedly accepted a missing vertex function"); ++ }; ++ assert!( ++ error.to_string().contains("missing_vertex_function"), ++ "unexpected pipeline error: {error:#}" ++ ); ++ Ok(()) ++ } ++ ++ #[test] ++ fn command_buffer_resources_follow_completion_or_scope_lifetime() { ++ let Some(device) = metal::Device::system_default() else { ++ return; ++ }; ++ let command_queue = device.new_command_queue(); ++ ++ let completed_drop = Arc::new(AtomicBool::new(false)); ++ let command_buffer = command_queue.new_command_buffer(); ++ let mut completed_resources = CommandBufferResources::default(); ++ completed_resources.push(DropProbe(completed_drop.clone())); ++ completed_resources.retain_until_completion(command_buffer); ++ assert!(!completed_drop.load(Ordering::Acquire)); ++ command_buffer.commit(); ++ command_buffer.wait_until_completed(); ++ assert!(completed_drop.load(Ordering::Acquire)); ++ ++ let unsubmitted_drop = Arc::new(AtomicBool::new(false)); ++ let mut unsubmitted_resources = CommandBufferResources::default(); ++ unsubmitted_resources.push(DropProbe(unsubmitted_drop.clone())); ++ assert!(!unsubmitted_drop.load(Ordering::Acquire)); ++ drop(unsubmitted_resources); ++ assert!(unsubmitted_drop.load(Ordering::Acquire)); ++ } ++ ++ #[test] ++ fn surface_error_logging_is_throttled() { ++ let last_log = std::time::Instant::now(); ++ let before_interval = last_log ++ .checked_add(SURFACE_ERROR_LOG_INTERVAL / 2) ++ .expect("test instant overflowed"); ++ let at_interval = last_log ++ .checked_add(SURFACE_ERROR_LOG_INTERVAL) ++ .expect("test instant overflowed"); ++ ++ assert!(surface_error_log_due(None, last_log)); ++ assert!(!surface_error_log_due(Some(last_log), before_interval)); ++ assert!(surface_error_log_due(Some(last_log), at_interval)); ++ } ++} ++ +@@ -2372 +2637,2 @@ mod backdrop_blur_tests { +- MetalRenderer::new_headless(Arc::new(Mutex::new(InstanceBufferPool::default()))); ++ MetalRenderer::new_headless(Arc::new(Mutex::new(InstanceBufferPool::default()))) ++ .expect("failed to initialize headless renderer"); +@@ -2454 +2720,4 @@ mod backdrop_blur_tests { - assert!(diff <= 3, "vignette or shift at window edge: max diff {diff}"); + assert!( + diff <= 3, @@ -295,10 +666,29 @@ index 97bad184dc..71a05aca4f 100644 + color.a *= surface_corner_alpha(input.position.xy, surfaces[0]); + return color; diff --git a/crates/gpui_macos/src/window.rs b/crates/gpui_macos/src/window.rs -index 8a3aa5ea06..e0a9b8e2ef 100644 +index 8a3aa5ea06..aacd697be5 100644 --- a/crates/gpui_macos/src/window.rs +++ b/crates/gpui_macos/src/window.rs -@@ -3135 +3135,2 @@ unsafe fn remove_layer_background(layer: id) { +@@ -7 +6,0 @@ use crate::{ +-#[cfg(any(test, feature = "test-support"))] +@@ -776 +775,3 @@ impl MacWindow { +- ) -> Self { ++ ) -> Result { ++ let renderer = ++ objc::rc::autoreleasepool(|| renderer::new_renderer(renderer_context, false))?; +@@ -896,7 +897 @@ impl MacWindow { +- renderer: renderer::new_renderer( +- renderer_context, +- native_window as *mut _, +- native_view as *mut _, +- bounds.size.map(|pixels| pixels.as_f32()), +- false, +- ), ++ renderer, +@@ -1101 +1096 @@ impl MacWindow { +- window ++ Ok(window) +@@ -3135 +3130,2 @@ unsafe fn remove_layer_background(layer: id) { - let _: () = msg_send![filter, setValue: radius forKey: ns_string("inputRadius")]; + let _: () = + msg_send![filter, setValue: radius forKey: ns_string("inputRadius")]; @@ -359,3 +749,25 @@ diff --git a/crates/gpui_windows/src/directx_atlas.rs b/crates/gpui_windows/src/ + ); + } +} +diff --git a/crates/gpui_macos/src/platform.rs b/crates/gpui_macos/src/platform.rs +index b25067935e..c60ef0b4ec 100644 +--- a/crates/gpui_macos/src/platform.rs ++++ b/crates/gpui_macos/src/platform.rs +@@ -669 +669 @@ impl Platform for MacPlatform { +- ))) ++ )?)) +diff --git a/crates/gpui_platform/src/gpui_platform.rs b/crates/gpui_platform/src/gpui_platform.rs +index 1d2fea90b4..c2e070e721 100644 +--- a/crates/gpui_platform/src/gpui_platform.rs ++++ b/crates/gpui_platform/src/gpui_platform.rs +@@ -67,3 +67,7 @@ pub fn current_headless_renderer() -> Option Some(Box::new(renderer)), ++ Err(error) => { ++ eprintln!("failed to initialize headless Metal renderer: {error:#}"); ++ None ++ } ++ } diff --git a/apps/desktop-gpui/src/main.rs b/apps/desktop-gpui/src/main.rs index 713f423227..0911ae9d53 100644 --- a/apps/desktop-gpui/src/main.rs +++ b/apps/desktop-gpui/src/main.rs @@ -254,7 +254,11 @@ fn main() { move |window, cx| cx.new(|cx| MainWindow::new(session, window, cx)) }, ) - .expect("failed to open the main window"); + .inspect_err(|error| tracing::error!("failed to open the main window: {error:#}")); + let Ok(window_handle) = window_handle else { + cx.quit(); + return; + }; app_windows::init(window_handle, session, cx); updates::schedule_startup_check(cx); From 40f194bf5d3bb40fe8b3ce051bdc74d5f3f55a7e Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:18:04 +0100 Subject: [PATCH 4/6] fix: recover from unexpected GPUI exits throughout the session --- apps/desktop-gpui/src/main.rs | 18 +++----- apps/desktop-gpui/src/menus.rs | 4 -- apps/desktop-gpui/src/store.rs | 63 ++++++++++++++++++++++---- apps/desktop/src-tauri/src/gpui_app.rs | 18 +++----- 4 files changed, 68 insertions(+), 35 deletions(-) diff --git a/apps/desktop-gpui/src/main.rs b/apps/desktop-gpui/src/main.rs index 0911ae9d53..1888df56d2 100644 --- a/apps/desktop-gpui/src/main.rs +++ b/apps/desktop-gpui/src/main.rs @@ -179,6 +179,7 @@ fn main() { // A relaunch means "run the code I just built": take over from any // previous instance still alive in the tray (see `single_instance`). single_instance::acquire(); + store::mark_handoff_session(); platform::install_url_scheme_handler(); for argument in std::env::args().skip(1) { @@ -260,6 +261,11 @@ fn main() { return; }; + cx.on_app_quit(|_| async { + crate::store::clear_handoff_marker(); + }) + .detach(); + app_windows::init(window_handle, session, cx); updates::schedule_startup_check(cx); @@ -289,18 +295,6 @@ fn main() { } }) .detach(); - // The other half of the Tauri app's hand-off protocol - // (`store::handoff_marker_path`): staying up this long is what proves - // to it that redirecting here was not a mistake. - cx.spawn(async move |cx| { - cx.background_executor() - .timer(std::time::Duration::from_secs(10)) - .await; - cx.background_executor() - .spawn(async { crate::store::clear_handoff_marker() }) - .await; - }) - .detach(); // `CAP_GPUI_AUTO_TRAY` / `CAP_GPUI_TRAY_DUMP`: the tray's harness path. tray::drive_from_env(cx); // `CAP_GPUI_AUTO_CLOSE=settings|main:`: run the ⌘W body against diff --git a/apps/desktop-gpui/src/menus.rs b/apps/desktop-gpui/src/menus.rs index d074e5abc3..19c1678e6a 100644 --- a/apps/desktop-gpui/src/menus.rs +++ b/apps/desktop-gpui/src/menus.rs @@ -287,10 +287,6 @@ pub fn close_window_by_handle(handle: gpui::AnyWindowHandle, cx: &mut App) { /// quit waits for the session to come back to `Idle` (bounded, so a wedged /// finalize cannot make the app unquittable). pub fn quit(cx: &mut App) { - // Reaching a deliberate quit is proof enough that this build came up, even - // if it happens inside the marker's ten-second window. - crate::store::clear_handoff_marker(); - let session = RecordingSession::global(cx); if session.read(cx).phase == Phase::Idle { cx.quit(); diff --git a/apps/desktop-gpui/src/store.rs b/apps/desktop-gpui/src/store.rs index 2a362c7db4..8e5543d04d 100644 --- a/apps/desktop-gpui/src/store.rs +++ b/apps/desktop-gpui/src/store.rs @@ -193,21 +193,31 @@ fn bundled_resource_dirs_for( /// The Tauri app's hand-off marker (`gpui_app.rs`). /// -/// It writes this file immediately before spawning this app and never deletes -/// it; this app deletes it once it has been alive long enough to count as -/// healthy (`main.rs`) or on a clean quit (`menus::quit`). A marker the Tauri -/// app still finds at startup therefore means the build it handed off to never -/// came up, so it clears `enableGpuiApp` and takes the session back rather than -/// bouncing the user into an app that does not start. +/// It survives for the entire session, including recording finalization, and +/// is removed only during clean app shutdown. A timer cannot establish health: +/// an idle native callback can crash after any startup grace period has elapsed. pub fn handoff_marker_path() -> PathBuf { app_data_dir().join("cap-gpui.handoff") } +pub fn mark_handoff_session() { + if let Err(error) = mark_handoff_session_at(&handoff_marker_path()) { + tracing::warn!("writing the hand-off marker: {error}"); + } +} + +fn mark_handoff_session_at(path: &Path) -> std::io::Result<()> { + write_update_handoff_at(path, &std::process::id().to_string()) +} + /// Best-effort: a marker that cannot be removed only costs one fallback to the /// Tauri app on the next launch. pub fn clear_handoff_marker() { - let path = handoff_marker_path(); - match std::fs::remove_file(&path) { + clear_handoff_marker_at(&handoff_marker_path()); +} + +fn clear_handoff_marker_at(path: &Path) { + match std::fs::remove_file(path) { Ok(()) => tracing::info!("cleared the hand-off marker"), Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} Err(error) => tracing::warn!("clearing the hand-off marker: {error}"), @@ -1949,6 +1959,43 @@ mod tests { ); } + #[test] + fn handoff_marker_survives_until_explicit_clean_shutdown() { + let directory = + std::env::temp_dir().join(format!("cap-gpui-handoff-session-{}", super::new_uuid_v4())); + let marker = directory.join("cap-gpui.handoff"); + + super::mark_handoff_session_at(&marker).unwrap(); + assert_eq!( + std::fs::read_to_string(&marker).unwrap(), + std::process::id().to_string() + ); + super::mark_handoff_session_at(&marker).unwrap(); + assert!(marker.exists()); + + super::clear_handoff_marker_at(&marker); + assert!(!marker.exists()); + super::clear_handoff_marker_at(&marker); + std::fs::remove_dir(directory).unwrap(); + } + + #[test] + fn handoff_marker_write_failure_leaves_existing_state_intact() { + let directory = + std::env::temp_dir().join(format!("cap-gpui-handoff-failure-{}", super::new_uuid_v4())); + std::fs::create_dir_all(&directory).unwrap(); + let blocked_parent = directory.join("not-a-directory"); + std::fs::write(&blocked_parent, "unchanged").unwrap(); + + assert!(super::mark_handoff_session_at(&blocked_parent.join("marker")).is_err()); + assert_eq!( + std::fs::read_to_string(&blocked_parent).unwrap(), + "unchanged" + ); + std::fs::remove_file(blocked_parent).unwrap(); + std::fs::remove_dir(directory).unwrap(); + } + #[test] fn bundled_resource_paths_follow_the_installed_executable() { let executable = std::path::Path::new("/Applications/Cap.app/Contents/MacOS/cap-gpui"); diff --git a/apps/desktop/src-tauri/src/gpui_app.rs b/apps/desktop/src-tauri/src/gpui_app.rs index 43de84e917..8f6d6f1ea8 100644 --- a/apps/desktop/src-tauri/src/gpui_app.rs +++ b/apps/desktop/src-tauri/src/gpui_app.rs @@ -17,10 +17,9 @@ //! A handoff that spawns a `cap-gpui` which then dies immediately would leave //! the user with no app at all, and with a setting that keeps redirecting away //! from the app that does work. So this side writes `cap-gpui.handoff` next to -//! the shared store before spawning, and `cap-gpui` deletes it once it has been -//! alive for ~10 seconds (`store::handoff_marker_path`, `main.rs`). A marker -//! still present at startup therefore means the last handoff never reached a -//! healthy instance: clear the flag, tell the user, and start normally. +//! the shared store before spawning, and `cap-gpui` deletes it only on clean +//! shutdown. With no live GPUI process, a surviving marker means the previous +//! session exited unexpectedly: clear the flag, tell the user, and start normally. use std::path::PathBuf; @@ -775,11 +774,8 @@ fn redirect_decision(app: &AppHandle) -> bool { return false; } - // A live instance is checked before the marker: within ten seconds of a - // successful handoff the marker is still legitimately on disk, and healing - // then would clear the flag and open this app next to a healthy native one. - // The marker's lifecycle stays with `cap-gpui` -- if that instance dies - // before proving itself, the marker survives it and the next launch heals. + // The marker exists throughout a live session. Check the process first so + // reopening Cap does not mistake a running native app for a crashed one. if let Some(pid) = running_instance_pid() { info!(pid, "Cap GPUI is already running; handing over to it"); #[cfg(any(target_os = "macos", windows))] @@ -793,7 +789,7 @@ fn redirect_decision(app: &AppHandle) -> bool { let marker = handoff_marker(); if marker.exists() { - warn!("the last hand-off to Cap GPUI never reported a healthy instance; taking back over"); + warn!("the last Cap GPUI session exited unexpectedly; taking back over"); let _ = std::fs::remove_file(&marker); if let Err(error) = GeneralSettingsStore::update(app, |settings| { settings.enable_gpui_app = false; @@ -805,7 +801,7 @@ fn redirect_decision(app: &AppHandle) -> bool { } app.dialog() .message( - "The native Cap app didn't start correctly last time, so the classic app has been restored.", + "The native Cap app exited unexpectedly last time, so the classic app has been restored.", ) .show(|_| {}); return false; From e49f06fb868297d30ec83e711f864ed36ba78fa1 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:39:44 +0100 Subject: [PATCH 5/6] fix: preserve graceful shutdown during GPUI permission relaunch --- apps/desktop-gpui/src/onboarding_window.rs | 2 +- apps/desktop-gpui/src/permissions.rs | 165 ++++++++++++++++++--- 2 files changed, 144 insertions(+), 23 deletions(-) diff --git a/apps/desktop-gpui/src/onboarding_window.rs b/apps/desktop-gpui/src/onboarding_window.rs index fee5632958..04f8a776f8 100644 --- a/apps/desktop-gpui/src/onboarding_window.rs +++ b/apps/desktop-gpui/src/onboarding_window.rs @@ -582,7 +582,7 @@ impl OnboardingWindow { .radius(px(8.)) .icon("icons/rotate-ccw.svg") .label("Relaunch Cap") - .on_click(cx.listener(|_, _, _, _| permissions::relaunch())), + .on_click(cx.listener(|_, _, _, cx| permissions::relaunch(cx))), ), ), ) diff --git a/apps/desktop-gpui/src/permissions.rs b/apps/desktop-gpui/src/permissions.rs index 759f38222f..d88f7d20df 100644 --- a/apps/desktop-gpui/src/permissions.rs +++ b/apps/desktop-gpui/src/permissions.rs @@ -263,36 +263,68 @@ pub fn open_permission_settings(permission: OSPermission) { /// Relaunch the app -- the Tauri onboarding offers this after sending the /// user to System Settings, because a fresh screen-recording or accessibility -/// grant often only takes effect in a new process. Spawns a detached shell -/// that reopens the bundle (or the bare binary in dev) after this process has -/// had a beat to exit, then quits. -pub fn relaunch() { +/// grant often only takes effect in a new process. The replacement must wait +/// for shutdown: Tauri otherwise sees this process alive and exits without +/// starting a replacement. +pub fn relaunch(cx: &mut gpui::App) { + static REQUESTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + let Ok(exe) = std::env::current_exe() else { tracing::error!("relaunch: current_exe unavailable"); return; }; + let mut command = relaunch_command(&exe, std::process::id()); + if let Err(error) = spawn_relaunch(&mut command, &REQUESTED, || crate::menus::quit(cx)) { + tracing::error!("relaunch failed to spawn: {error}"); + } +} + +fn relaunch_command(exe: &std::path::Path, pid: u32) -> std::process::Command { #[cfg(target_os = "macos")] - let command = { - let bundle = exe - .ancestors() - .find(|path| path.extension().is_some_and(|ext| ext == "app")) - .map(std::path::Path::to_path_buf); - match bundle { - Some(bundle) => format!("sleep 0.3; /usr/bin/open -n \"{}\"", bundle.display()), - None => format!("sleep 0.3; exec \"{}\"", exe.display()), - } - }; + let bundle = exe + .ancestors() + .find(|path| path.extension().is_some_and(|ext| ext == "app")); #[cfg(not(target_os = "macos"))] - let command = format!("sleep 0.3; exec \"{}\"", exe.display()); + let bundle: Option<&std::path::Path> = None; + + let (script, target) = match bundle { + Some(bundle) => ( + r#"while kill -0 "$1" 2>/dev/null; do sleep 0.1; done; exec /usr/bin/open -n "$2""#, + bundle, + ), + None => ( + r#"while kill -0 "$1" 2>/dev/null; do sleep 0.1; done; exec "$2""#, + exe, + ), + }; + let mut command = std::process::Command::new("/bin/sh"); + command + .args(["-c", script, "cap-relaunch"]) + .arg(pid.to_string()) + .arg(target); + command +} - match std::process::Command::new("/bin/sh") - .arg("-c") - .arg(command) - .spawn() - { - Ok(_) => std::process::exit(0), - Err(error) => tracing::error!("relaunch failed to spawn: {error}"), +fn spawn_relaunch( + command: &mut std::process::Command, + requested: &std::sync::atomic::AtomicBool, + quit: impl FnOnce(), +) -> std::io::Result> { + use std::sync::atomic::Ordering; + + if requested.swap(true, Ordering::AcqRel) { + return Ok(None); + } + match command.spawn() { + Ok(child) => { + quit(); + Ok(Some(child)) + } + Err(error) => { + requested.store(false, Ordering::Release); + Err(error) + } } } @@ -401,6 +433,95 @@ mod macos { mod tests { use super::*; + #[test] + fn relaunch_requests_quit_once_after_spawn_and_allows_retry_after_failure() { + use std::{ + cell::Cell, + sync::atomic::{AtomicBool, Ordering}, + }; + + let requested = AtomicBool::new(false); + let quits = Cell::new(0); + assert!( + spawn_relaunch(&mut std::process::Command::new(""), &requested, || { + quits.set(quits.get() + 1); + }) + .is_err() + ); + assert!(!requested.load(Ordering::Acquire)); + assert_eq!(quits.get(), 0); + + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + command.arg("--list").stdout(std::process::Stdio::null()); + let mut child = spawn_relaunch(&mut command, &requested, || quits.set(quits.get() + 1)) + .unwrap() + .unwrap(); + assert!(child.wait().unwrap().success()); + assert!( + spawn_relaunch(&mut command, &requested, || quits.set(quits.get() + 1)) + .unwrap() + .is_none() + ); + assert_eq!(quits.get(), 1); + } + + #[cfg(target_os = "macos")] + #[test] + fn bundled_relaunch_passes_the_bundle_as_a_literal_argument() { + let bundle = std::path::Path::new("/Applications/Cap ' \" $ app.app"); + let command = relaunch_command(&bundle.join("Contents/MacOS/cap-gpui"), 123); + let args = command.get_args().collect::>(); + assert_eq!(command.get_program(), "/bin/sh"); + assert_eq!(args[3], "123"); + assert_eq!(args[4], bundle.as_os_str()); + assert!( + args[1] + .to_str() + .unwrap() + .ends_with(r#"exec /usr/bin/open -n "$2""#) + ); + } + + #[cfg(unix)] + #[test] + fn relaunch_waits_for_the_previous_process_and_handles_literal_paths() { + use std::os::unix::fs::PermissionsExt; + + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let root = + std::env::temp_dir().join(format!("cap-gpui-relaunch-{}-{nonce}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let executable = root.join("replacement ' \" $ program"); + let output = root.join("relaunched"); + std::fs::write( + &executable, + b"#!/bin/sh\nprintf relaunched > \"$CAP_GPUI_RELAUNCH_TEST_OUTPUT\"\n", + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o700)).unwrap(); + + let mut previous = std::process::Command::new("/bin/sh") + .args(["-c", "read -r line"]) + .stdin(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let mut replacement = relaunch_command(&executable, previous.id()) + .env("CAP_GPUI_RELAUNCH_TEST_OUTPUT", &output) + .spawn() + .unwrap(); + std::thread::sleep(std::time::Duration::from_millis(400)); + assert!(replacement.try_wait().unwrap().is_none()); + assert!(!output.exists()); + drop(previous.stdin.take()); + previous.wait().unwrap(); + assert!(replacement.wait().unwrap().success()); + assert_eq!(std::fs::read(&output).unwrap(), b"relaunched"); + std::fs::remove_dir_all(root).unwrap(); + } + fn raw( screen: bool, ax: bool, From a1258e58b9681d6f0785f5f007326f543b776f45 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:10:31 +0100 Subject: [PATCH 6/6] fix: embed the muted volume icon --- apps/desktop-gpui/src/assets.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/desktop-gpui/src/assets.rs b/apps/desktop-gpui/src/assets.rs index 28b67ca0d5..7af985b8a7 100644 --- a/apps/desktop-gpui/src/assets.rs +++ b/apps/desktop-gpui/src/assets.rs @@ -176,6 +176,7 @@ const ICONS: &[(&str, &[u8])] = assets!("icons": "sparkles.svg", "timer.svg", "volume-2.svg", + "volume-x.svg", "diamond.svg", "x-mark.svg", "zap.svg",