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.
This commit is contained in:
1 parent
3372e25a2c
commit
c56dcef972
5 files changed
+42
-2
No files matched your search
Generated
+2
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<bool> = 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<T>(closure: impl FnOnce() -> T) -> anyhow::Result<T> {
|
||||
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::<String>().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<Arc<Client>> = 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("<unnamed>");
|
||||
|
||||
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);
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user