From a098b3e93dd2f1ad5a44ab56491bad4ff5b04521 Mon Sep 17 00:00:00 2001 From: Marco Allegretti Date: Fri, 13 Mar 2026 14:01:42 +0100 Subject: [PATCH] fix(shell): harden Servo shell and app shell against rendering and lifecycle failures - blit_software: replace expect() on softbuffer Context and Surface creation with log-and-skip so a frame failure does not crash the process - blit_software: replace NonZeroU32::new(1).unwrap() with NonZeroU32::MIN - resumed: replace create_window().expect() with a match that calls event_loop.exit() and returns on failure instead of unwinding - build_rendering_ctx: return Option instead of panicking when SoftwareRenderingContext creation fails; callers exit cleanly - resumed (app-shell): exit without emitting READY when no rendering context is available so weft-appd observes a clean session failure - weft-servo-shell: bound gesture forwarding to one active thread at a time using JoinHandle::is_finished(); excess batches are dropped with a debug log to prevent unbounded thread creation per event loop tick - shell_client (both shells): replace post-ensure! unwrap() with expect() that documents the invariant --- crates/weft-app-shell/src/embedder.rs | 50 +++++++++----- crates/weft-app-shell/src/shell_client.rs | 5 +- crates/weft-servo-shell/src/embedder.rs | 72 +++++++++++++++------ crates/weft-servo-shell/src/shell_client.rs | 5 +- 4 files changed, 92 insertions(+), 40 deletions(-) diff --git a/crates/weft-app-shell/src/embedder.rs b/crates/weft-app-shell/src/embedder.rs index 0cff4d2..6fae473 100644 --- a/crates/weft-app-shell/src/embedder.rs +++ b/crates/weft-app-shell/src/embedder.rs @@ -130,12 +130,17 @@ impl App { let Some(pixels) = rendering_context.read_pixels() else { return; }; - let ctx = softbuffer::Context::new(Arc::clone(window)).expect("softbuffer context"); - let mut surface = - softbuffer::Surface::new(&ctx, Arc::clone(window)).expect("softbuffer surface"); + let Ok(ctx) = softbuffer::Context::new(Arc::clone(window)) else { + tracing::warn!("softbuffer context creation failed; skipping frame"); + return; + }; + let Ok(mut surface) = softbuffer::Surface::new(&ctx, Arc::clone(window)) else { + tracing::warn!("softbuffer surface creation failed; skipping frame"); + return; + }; let _ = surface.resize( - std::num::NonZeroU32::new(size.width).unwrap_or(std::num::NonZeroU32::new(1).unwrap()), - std::num::NonZeroU32::new(size.height).unwrap_or(std::num::NonZeroU32::new(1).unwrap()), + std::num::NonZeroU32::new(size.width).unwrap_or(std::num::NonZeroU32::MIN), + std::num::NonZeroU32::new(size.height).unwrap_or(std::num::NonZeroU32::MIN), ); let Ok(mut buf) = surface.buffer_mut() else { return; @@ -155,11 +160,14 @@ impl ApplicationHandler for App { let title = format!("WEFT App — {}", self.url.host_str().unwrap_or("app")); let attrs = WindowAttributes::default().with_title(title); - let window = Arc::new( - event_loop - .create_window(attrs) - .expect("failed to create app window"), - ); + let window = match event_loop.create_window(attrs) { + Ok(w) => Arc::new(w), + Err(e) => { + tracing::error!(error = %e, "failed to create app window; exiting"); + event_loop.exit(); + return; + } + }; let size = window.inner_size(); self.window = Some(Arc::clone(&window)); @@ -185,7 +193,11 @@ impl ApplicationHandler for App { servo.set_delegate(Rc::new(WeftServoDelegate)); - let rendering_context = build_rendering_ctx(event_loop, &window, size); + let Some(rendering_context) = build_rendering_ctx(event_loop, &window, size) else { + tracing::error!("no rendering context available; exiting without READY signal"); + event_loop.exit(); + return; + }; let ucm = Rc::new(UserContentManager::new(&servo)); if let Some(kit_js) = load_ui_kit_script() { @@ -326,7 +338,7 @@ fn build_rendering_ctx( event_loop: &ActiveEventLoop, window: &Arc, size: winit::dpi::PhysicalSize, -) -> RenderingCtx { +) -> Option { if std::env::var_os("WEFT_EGL_RENDERING").is_some() { let display_handle = event_loop.display_handle(); let window_handle = window.window_handle(); @@ -334,7 +346,7 @@ fn build_rendering_ctx( match servo::WindowRenderingContext::new(dh, wh, size) { Ok(rc) => { tracing::info!("using EGL rendering context"); - return RenderingCtx::Egl(Rc::new(rc)); + return Some(RenderingCtx::Egl(Rc::new(rc))); } Err(e) => { tracing::warn!("EGL rendering context failed ({e}), falling back to software"); @@ -342,10 +354,14 @@ fn build_rendering_ctx( } } } - RenderingCtx::Software(Rc::new( - servo::SoftwareRenderingContext::new(servo::euclid::Size2D::new(size.width, size.height)) - .expect("SoftwareRenderingContext"), - )) + match servo::SoftwareRenderingContext::new(servo::euclid::Size2D::new(size.width, size.height)) + { + Ok(rc) => Some(RenderingCtx::Software(Rc::new(rc))), + Err(e) => { + tracing::error!("SoftwareRenderingContext failed: {e}"); + None + } + } } fn ui_kit_path() -> std::path::PathBuf { diff --git a/crates/weft-app-shell/src/shell_client.rs b/crates/weft-app-shell/src/shell_client.rs index d5839b0..58d8918 100644 --- a/crates/weft-app-shell/src/shell_client.rs +++ b/crates/weft-app-shell/src/shell_client.rs @@ -172,7 +172,10 @@ impl ShellClient { WlSurface::from_id(&conn, id).context("wl_surface from_id")? }; - let manager = data.manager.as_ref().unwrap(); + let manager = data + .manager + .as_ref() + .expect("manager is Some; guaranteed by ensure! above"); let title = format!("{app_id}/{session_id}"); let window = manager.create_window( app_id.to_string(), diff --git a/crates/weft-servo-shell/src/embedder.rs b/crates/weft-servo-shell/src/embedder.rs index f001817..6d7e875 100644 --- a/crates/weft-servo-shell/src/embedder.rs +++ b/crates/weft-servo-shell/src/embedder.rs @@ -96,6 +96,7 @@ struct App { modifiers: ModifiersState, cursor_pos: servo::euclid::default::Point2D, shell_client: Option, + gesture_thread: Option>, } impl App { @@ -113,6 +114,7 @@ impl App { modifiers: ModifiersState::default(), cursor_pos: servo::euclid::default::Point2D::zero(), shell_client: None, + gesture_thread: None, } } @@ -128,12 +130,17 @@ impl App { let Some(pixels) = rendering_context.read_pixels() else { return; }; - let ctx = softbuffer::Context::new(Arc::clone(window)).expect("softbuffer context"); - let mut surface = - softbuffer::Surface::new(&ctx, Arc::clone(window)).expect("softbuffer surface"); + let Ok(ctx) = softbuffer::Context::new(Arc::clone(window)) else { + tracing::warn!("softbuffer context creation failed; skipping frame"); + return; + }; + let Ok(mut surface) = softbuffer::Surface::new(&ctx, Arc::clone(window)) else { + tracing::warn!("softbuffer surface creation failed; skipping frame"); + return; + }; let _ = surface.resize( - std::num::NonZeroU32::new(size.width).unwrap_or(std::num::NonZeroU32::new(1).unwrap()), - std::num::NonZeroU32::new(size.height).unwrap_or(std::num::NonZeroU32::new(1).unwrap()), + std::num::NonZeroU32::new(size.width).unwrap_or(std::num::NonZeroU32::MIN), + std::num::NonZeroU32::new(size.height).unwrap_or(std::num::NonZeroU32::MIN), ); let Ok(mut buf) = surface.buffer_mut() else { return; @@ -152,11 +159,14 @@ impl ApplicationHandler for App { } let attrs = WindowAttributes::default().with_title("WEFT Shell"); - let window = Arc::new( - event_loop - .create_window(attrs) - .expect("failed to create shell window"), - ); + let window = match event_loop.create_window(attrs) { + Ok(w) => Arc::new(w), + Err(e) => { + tracing::error!(error = %e, "failed to create shell window; exiting"); + event_loop.exit(); + return; + } + }; let size = window.inner_size(); self.window = Some(Arc::clone(&window)); @@ -177,7 +187,11 @@ impl ApplicationHandler for App { servo.set_delegate(Rc::new(WeftServoDelegate)); - let rendering_context = build_rendering_ctx(event_loop, &window, size); + let Some(rendering_context) = build_rendering_ctx(event_loop, &window, size) else { + tracing::error!("no rendering context available; shell cannot start"); + event_loop.exit(); + return; + }; let user_content_manager = Rc::new(UserContentManager::new(&servo)); if let Some(kit_js) = load_ui_kit_script() { @@ -226,10 +240,22 @@ impl ApplicationHandler for App { } let gestures = sc.take_pending_gestures(); if !gestures.is_empty() { - let ws_port = self.ws_port; - std::thread::spawn(move || { - forward_gestures_to_appd(ws_port, &gestures); - }); + let prev_done = self + .gesture_thread + .as_ref() + .map(|h| h.is_finished()) + .unwrap_or(true); + if prev_done { + let ws_port = self.ws_port; + self.gesture_thread = Some(std::thread::spawn(move || { + forward_gestures_to_appd(ws_port, &gestures); + })); + } else { + tracing::debug!( + count = gestures.len(), + "gesture forwarding in progress; dropping batch" + ); + } } } if let Some(servo) = &self.servo { @@ -319,7 +345,7 @@ fn build_rendering_ctx( event_loop: &ActiveEventLoop, window: &Arc, size: winit::dpi::PhysicalSize, -) -> RenderingCtx { +) -> Option { if std::env::var_os("WEFT_EGL_RENDERING").is_some() { let display_handle = event_loop.display_handle(); let window_handle = window.window_handle(); @@ -327,7 +353,7 @@ fn build_rendering_ctx( match servo::WindowRenderingContext::new(dh, wh, size) { Ok(rc) => { tracing::info!("using EGL rendering context"); - return RenderingCtx::Egl(Rc::new(rc)); + return Some(RenderingCtx::Egl(Rc::new(rc))); } Err(e) => { tracing::warn!("EGL rendering context failed ({e}), falling back to software"); @@ -335,10 +361,14 @@ fn build_rendering_ctx( } } } - RenderingCtx::Software(Rc::new( - servo::SoftwareRenderingContext::new(servo::euclid::Size2D::new(size.width, size.height)) - .expect("SoftwareRenderingContext"), - )) + match servo::SoftwareRenderingContext::new(servo::euclid::Size2D::new(size.width, size.height)) + { + Ok(rc) => Some(RenderingCtx::Software(Rc::new(rc))), + Err(e) => { + tracing::error!("SoftwareRenderingContext failed: {e}"); + None + } + } } // ── Public entry point ──────────────────────────────────────────────────────── diff --git a/crates/weft-servo-shell/src/shell_client.rs b/crates/weft-servo-shell/src/shell_client.rs index e26d876..b57ab85 100644 --- a/crates/weft-servo-shell/src/shell_client.rs +++ b/crates/weft-servo-shell/src/shell_client.rs @@ -197,7 +197,10 @@ impl ShellClient { WlSurface::from_id(&conn, id).context("wl_surface from_id")? }; - let manager = data.manager.as_ref().unwrap(); + let manager = data + .manager + .as_ref() + .expect("manager is Some; guaranteed by ensure! above"); let window = manager.create_window( "org.weft.system.shell".to_string(), "WEFT Shell".to_string(),