From c56dcef972d0550136eb084fda84adafbbcab280 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 a9aa6e2303..39826863f2 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 5e451853a9..3523426752 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 a1a43dbb88..99ec14fc04 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 55912c66a0..1cfc1b4e59 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 4d26b7e895..67131a6b2c 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)