crashes: Avoid crash handler on detached threads (#40883)

Nia and dino created 1 week ago

Set a TLS bit to skip invoking the crash handler when a detached thread
panics.

cc @P1n3appl3 - is this at odds with what we need the crash handler to
do?

May close #39289, cannot repro without a nightly build

Release Notes:

- Fixed extension panics crashing Zed on Linux

Co-authored-by: dino <[e-mail]>

Change summary

Cargo.lock                             |  1 +
crates/crashes/Cargo.toml              |  1 +
crates/crashes/src/crashes.rs          |  5 +++++
crates/extension_host/src/wasm_host.rs | 16 ++++++++++++----
crates/gpui/src/executor.rs            | 20 ++++++++++++--------
5 files changed, 31 insertions(+), 12 deletions(-)

Detailed changes

Cargo.lock

@@ -4132,6 +4132,7 @@ dependencies = [
  "bincode 1.3.3",
  "cfg-if",
  "crash-handler",
+ "extension_host",
  "log",
  "mach2 0.5.0",
  "minidumper",

crates/crashes/Cargo.toml

@@ -9,6 +9,7 @@ license = "GPL-3.0-or-later"
 bincode.workspace = true
 cfg-if.workspace = true
 crash-handler.workspace = true
+extension_host.workspace = true
 log.workspace = true
 minidumper.workspace = true
 paths.workspace = true

crates/crashes/src/crashes.rs

@@ -286,6 +286,11 @@ impl minidumper::ServerHandler for CrashServer {
 }
 
 pub fn panic_hook(info: &PanicHookInfo) {
+    // Don't handle a panic on threads that are not relevant to the main execution.
+    if extension_host::wasm_host::IS_WASM_THREAD.with(|v| v.load(Ordering::Acquire)) {
+        return;
+    }
+
     let message = info
         .payload()
         .downcast_ref::<&str>()

crates/extension_host/src/wasm_host.rs

@@ -30,12 +30,14 @@ use node_runtime::NodeRuntime;
 use release_channel::ReleaseChannel;
 use semantic_version::SemanticVersion;
 use settings::Settings;
-use std::borrow::Cow;
-use std::sync::{LazyLock, OnceLock};
-use std::time::Duration;
 use std::{
+    borrow::Cow,
     path::{Path, PathBuf},
-    sync::Arc,
+    sync::{
+        Arc, LazyLock, OnceLock,
+        atomic::{AtomicBool, Ordering},
+    },
+    time::Duration,
 };
 use task::{DebugScenario, SpawnInTerminal, TaskTemplate, ZedDebugConfig};
 use util::paths::SanitizedPath;
@@ -495,6 +497,11 @@ pub struct WasmState {
     pub(crate) capability_granter: CapabilityGranter,
 }
 
+std::thread_local! {
+    /// Used by the crash handler to ignore panics in extension-related threads.
+    pub static IS_WASM_THREAD: AtomicBool = const { AtomicBool::new(false) };
+}
+
 type MainThreadCall = Box<dyn Send + for<'a> FnOnce(&'a mut AsyncApp) -> LocalBoxFuture<'a, ()>>;
 
 type ExtensionCall = Box<
@@ -529,6 +536,7 @@ fn wasm_engine(executor: &BackgroundExecutor) -> wasmtime::Engine {
             let engine_ref = engine.weak();
             executor
                 .spawn(async move {
+                    IS_WASM_THREAD.with(|v| v.store(true, Ordering::Release));
                     // Somewhat arbitrary interval, as it isn't a guaranteed interval.
                     // But this is a rough upper bound for how long the extension execution can block on
                     // `Future::poll`.

crates/gpui/src/executor.rs

@@ -2,21 +2,20 @@ use crate::{App, PlatformDispatcher};
 use async_task::Runnable;
 use futures::channel::mpsc;
 use smol::prelude::*;
-use std::mem::ManuallyDrop;
-use std::panic::Location;
-use std::thread::{self, ThreadId};
 use std::{
     fmt::Debug,
     marker::PhantomData,
-    mem,
+    mem::{self, ManuallyDrop},
     num::NonZeroUsize,
+    panic::Location,
     pin::Pin,
     rc::Rc,
     sync::{
         Arc,
-        atomic::{AtomicUsize, Ordering::SeqCst},
+        atomic::{AtomicUsize, Ordering},
     },
     task::{Context, Poll},
+    thread::{self, ThreadId},
     time::{Duration, Instant},
 };
 use util::TryFutureExt;
@@ -123,7 +122,12 @@ impl TaskLabel {
     /// Construct a new task label.
     pub fn new() -> Self {
         static NEXT_TASK_LABEL: AtomicUsize = AtomicUsize::new(1);
-        Self(NEXT_TASK_LABEL.fetch_add(1, SeqCst).try_into().unwrap())
+        Self(
+            NEXT_TASK_LABEL
+                .fetch_add(1, Ordering::SeqCst)
+                .try_into()
+                .unwrap(),
+        )
     }
 }
 
@@ -271,7 +275,7 @@ impl BackgroundExecutor {
             let awoken = awoken.clone();
             let unparker = unparker.clone();
             move || {
-                awoken.store(true, SeqCst);
+                awoken.store(true, Ordering::SeqCst);
                 unparker.unpark();
             }
         });
@@ -287,7 +291,7 @@ impl BackgroundExecutor {
                     max_ticks -= 1;
 
                     if !dispatcher.tick(background_only) {
-                        if awoken.swap(false, SeqCst) {
+                        if awoken.swap(false, Ordering::SeqCst) {
                             continue;
                         }