From a25d1249df836e9e7779632e5f77cbfdb4e1732f Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Thu, 26 Feb 2026 15:41:27 +0530 Subject: [PATCH] markdown_preview: Fix panic in mermaid diagram renderer (#50176) The mermaid renderer can fail to render certain diagrams, and we already have a fallback for that. It's not worth crashing Zed over it. Wrapping `catch_unwind` alone doesn't work because our panic hooks terminate the process before unwinding begins (we intentionally terminate so the crash handler subprocess can generate a minidump), so there was no way to gracefully handle panics from third-party code until now (in rare cases where we need it, like in this case). To handle this gracefully, we added `crashes::recoverable_panic` which tells the panic hook to stand down on the current thread so the unwind can proceed and be caught. This should be used sparingly since caught panics bypass crash reporting. Release Notes: - Fixed a crash when rendering mermaid diagrams in markdown preview. --- Cargo.lock | 2 ++ crates/crashes/Cargo.toml | 1 + crates/crashes/src/crashes.rs | 36 ++++++++++++++++++- crates/markdown_preview/Cargo.toml | 1 + .../markdown_preview/src/markdown_renderer.rs | 4 ++- 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9aa6e2303ce4bf15ffe35b0a1a1948e0950d06a..39826863f2f483b73475a5f325ee8bc3e5f70801 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4081,6 +4081,7 @@ dependencies = [ name = "crashes" version = "0.1.0" dependencies = [ + "anyhow", "bincode", "cfg-if", "crash-handler", @@ -9994,6 +9995,7 @@ dependencies = [ "anyhow", "async-recursion", "collections", + "crashes", "editor", "fs", "gpui", diff --git a/crates/crashes/Cargo.toml b/crates/crashes/Cargo.toml index 5e451853a925d86ffcc1491a5c95af1f94e6ed05..3523426752e670c2c1023a1e0af221029f501070 100644 --- a/crates/crashes/Cargo.toml +++ b/crates/crashes/Cargo.toml @@ -6,6 +6,7 @@ edition.workspace = true license = "GPL-3.0-or-later" [dependencies] +anyhow.workspace = true bincode.workspace = true cfg-if.workspace = true crash-handler.workspace = true diff --git a/crates/crashes/src/crashes.rs b/crates/crashes/src/crashes.rs index a1a43dbb88198b7afd4b89141f7578c0a5bc25ce..99ec14fc049d359f1720a3e0605bc4597ceadbbe 100644 --- a/crates/crashes/src/crashes.rs +++ b/crates/crashes/src/crashes.rs @@ -4,6 +4,7 @@ use log::info; use minidumper::{Client, LoopAction, MinidumpBinary}; use release_channel::{RELEASE_CHANNEL, ReleaseChannel}; use serde::{Deserialize, Serialize}; +use std::cell::Cell; use std::mem; #[cfg(not(target_os = "windows"))] @@ -15,7 +16,7 @@ use std::{ env, fs::{self, File}, io, - panic::{self, PanicHookInfo}, + panic::{self, AssertUnwindSafe, PanicHookInfo}, path::{Path, PathBuf}, process::{self}, sync::{ @@ -26,6 +27,31 @@ use std::{ time::Duration, }; +thread_local! { + static ALLOW_UNWIND: Cell = const { Cell::new(false) }; +} + +/// Catch a panic as an error instead of aborting the process. Unlike plain +/// `catch_unwind`, this bypasses the crash-reporting panic hook which would +/// normally abort before unwinding can occur. +/// +/// **Use sparingly.** Prefer this only for isolating third-party code +/// that is known to panic, where you want to handle the failure gracefully +/// instead of crashing. +pub fn recoverable_panic(closure: impl FnOnce() -> T) -> anyhow::Result { + ALLOW_UNWIND.with(|flag| flag.set(true)); + let result = panic::catch_unwind(AssertUnwindSafe(closure)); + ALLOW_UNWIND.with(|flag| flag.set(false)); + result.map_err(|payload| { + let message = payload + .downcast_ref::<&str>() + .map(|s| s.to_string()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "unknown panic".to_string()); + anyhow::anyhow!("panic: {message}") + }) +} + // set once the crash handler has initialized and the client has connected to it pub static CRASH_HANDLER: OnceLock> = OnceLock::new(); // set when the first minidump request is made to avoid generating duplicate crash reports @@ -57,6 +83,9 @@ pub fn init(crash_init: InitCrashHandler, spawn: impl FnOnce(BoxFuture<'static, if !should_install_crash_handler() { let old_hook = panic::take_hook(); panic::set_hook(Box::new(move |info| { + if ALLOW_UNWIND.with(|flag| flag.get()) { + return; + } unsafe { env::set_var("RUST_BACKTRACE", "1") }; old_hook(info); // prevent the macOS crash dialog from popping up @@ -322,6 +351,11 @@ pub fn panic_hook(info: &PanicHookInfo) { let current_thread = std::thread::current(); let thread_name = current_thread.name().unwrap_or(""); + if ALLOW_UNWIND.with(|flag| flag.get()) { + log::error!("thread '{thread_name}' panicked at {span} (allowing unwind):\n{message}"); + return; + } + // wait 500ms for the crash handler process to start up // if it's still not there just write panic info and no minidump let retry_frequency = Duration::from_millis(100); diff --git a/crates/markdown_preview/Cargo.toml b/crates/markdown_preview/Cargo.toml index 55912c66a017fa22902f9b05e5fa924230710d69..1cfc1b4e59ef14b47ab5845dc67e2ad77c9232e5 100644 --- a/crates/markdown_preview/Cargo.toml +++ b/crates/markdown_preview/Cargo.toml @@ -18,6 +18,7 @@ test-support = [] anyhow.workspace = true async-recursion.workspace = true collections.workspace = true +crashes.workspace = true editor.workspace = true fs.workspace = true gpui.workspace = true diff --git a/crates/markdown_preview/src/markdown_renderer.rs b/crates/markdown_preview/src/markdown_renderer.rs index 4d26b7e8958a04f1bb64abc5be5502e23896f313..67131a6b2cb81f82a2c550944c96fb4e1ed5a93a 100644 --- a/crates/markdown_preview/src/markdown_renderer.rs +++ b/crates/markdown_preview/src/markdown_renderer.rs @@ -133,7 +133,9 @@ impl CachedMermaidDiagram { let _task = cx.spawn(async move |this, cx| { let value = cx .background_spawn(async move { - let svg_string = mermaid_rs_renderer::render(&contents.contents)?; + let svg_string = crashes::recoverable_panic(|| { + mermaid_rs_renderer::render(&contents.contents) + })??; let scale = contents.scale as f32 / 100.0; svg_renderer .render_single_frame(svg_string.as_bytes(), scale, true)