mirror of
https://github.com/zed-industries/zed.git
synced 2026-07-10 00:13:29 +00:00
Move the crash handler process spawn to a background thread (#58881)
Moves the crash handler subprocess spawn off the main thread to speed up startup, especially on Windows where process creation is slower. Previously `crashes::init` did part of its work synchronously when it was *called*, including `spawn_crash_handler`, which launches the `zed --crash-handler` child process (a synchronous `CreateProcessW` on Windows). Because `init` is evaluated inline as the argument to `background_executor().spawn(crashes::init(...))` in `main.rs`, that subprocess spawn ran on the main thread during startup rather than on the executor. This PR makes `connect_and_keepalive` a fully `async fn`, so all of that work, including the subprocess spawn, now runs on the background executor instead of blocking the main thread. I don't have a Windows machine to capture before/after numbers, but the crash handler spawn is clearly on the startup critical path. Logs in the Windows slow startup reports show it taking ~100–700ms between `spawning crash handler process` and `connected to crash handler process` (e.g. #40621, #54856), all of which previously blocked the main thread. Related to #49442 Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Improved startup performance
This commit is contained in:
Anthony Eid
GitHub
parent
a225d51024
commit
253606e8e0
1 changed files with 86 additions and 87 deletions
|
|
@ -41,9 +41,9 @@ pub fn force_backtrace() {
|
|||
|
||||
/// Install crash signal handlers and spawn the crash-handler subprocess.
|
||||
///
|
||||
/// The synchronous portion (signal handlers, panic hook) runs inline.
|
||||
/// The async keepalive task is passed to `spawn` so the caller decides
|
||||
/// which executor to schedule it on.
|
||||
/// All work happens lazily in the returned future, so it runs on whichever
|
||||
/// executor polls it. The keepalive task is passed to `spawn` so the caller
|
||||
/// decides which executor to schedule it on.
|
||||
pub fn init<F, S, C, P>(
|
||||
crash_init: InitCrashHandler,
|
||||
spawn: S,
|
||||
|
|
@ -60,13 +60,14 @@ where
|
|||
}
|
||||
|
||||
/// Spawn the crash-handler subprocess, connect the IPC client, and run the
|
||||
/// keepalive ping loop. Called on a background executor by [`init`].
|
||||
fn connect_and_keepalive<F, C, S, P>(
|
||||
/// keepalive ping loop. This is the future returned by [`init`], so it runs on
|
||||
/// whichever executor the caller polls it with.
|
||||
async fn connect_and_keepalive<F, C, S, P>(
|
||||
crash_init: InitCrashHandler,
|
||||
socket_path: P,
|
||||
wait_timer: C,
|
||||
spawn: S,
|
||||
) -> impl Future<Output = Arc<Client>> + use<F, C, S, P>
|
||||
) -> Arc<Client>
|
||||
where
|
||||
F: Future<Output = ()> + Send + Sync + 'static,
|
||||
C: (Fn(Duration) -> F) + Send + Sync + 'static,
|
||||
|
|
@ -77,93 +78,91 @@ where
|
|||
let socket_path = socket_path(process::id());
|
||||
let mut _crash_handler = spawn_crash_handler(&exe, &socket_path);
|
||||
info!("spawning crash handler process");
|
||||
async move {
|
||||
let mut elapsed = Duration::ZERO;
|
||||
let retry_frequency = Duration::from_millis(100);
|
||||
let client = loop {
|
||||
if let Ok(client) = Client::with_name(SocketName::Path(&socket_path)) {
|
||||
info!("connected to crash handler process after {elapsed:?}");
|
||||
break client;
|
||||
}
|
||||
elapsed += retry_frequency;
|
||||
wait_timer(retry_frequency).await;
|
||||
};
|
||||
let client = Arc::new(client);
|
||||
let mut elapsed = Duration::ZERO;
|
||||
let retry_frequency = Duration::from_millis(100);
|
||||
let client = loop {
|
||||
if let Ok(client) = Client::with_name(SocketName::Path(&socket_path)) {
|
||||
info!("connected to crash handler process after {elapsed:?}");
|
||||
break client;
|
||||
}
|
||||
elapsed += retry_frequency;
|
||||
wait_timer(retry_frequency).await;
|
||||
};
|
||||
let client = Arc::new(client);
|
||||
|
||||
panic::set_hook({
|
||||
let client = client.clone();
|
||||
Box::new(move |payload| {
|
||||
panic_hook(
|
||||
client.clone(),
|
||||
payload.payload_as_str().unwrap_or("Box<Any>"),
|
||||
payload.location(),
|
||||
)
|
||||
})
|
||||
});
|
||||
info!("panic handler registered");
|
||||
let handler = CrashHandler::attach(unsafe {
|
||||
let client = client.clone();
|
||||
let handler = move |crash_context: &crash_handler::CrashContext| {
|
||||
// set when the first minidump request is made to avoid generating duplicate crash reports
|
||||
static REQUESTED_MINIDUMP: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
// only request a minidump once
|
||||
let res = if REQUESTED_MINIDUMP
|
||||
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
|
||||
.is_ok()
|
||||
{
|
||||
#[cfg(target_os = "macos")]
|
||||
macos::suspend_all_other_threads();
|
||||
|
||||
// on macos this "ping" is needed to ensure that all our
|
||||
// `client.send_message` calls have been processed before we trigger the
|
||||
// minidump request.
|
||||
client.ping().ok();
|
||||
let r = client.request_dump(crash_context);
|
||||
if let Err(e) = &r {
|
||||
eprintln!("failed to request dump: {:?}", e);
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
macos::resume_all_other_threads();
|
||||
r.is_ok()
|
||||
} else {
|
||||
true
|
||||
};
|
||||
CrashEventResult::Handled(res)
|
||||
};
|
||||
crash_handler::make_crash_event(handler)
|
||||
panic::set_hook({
|
||||
let client = client.clone();
|
||||
Box::new(move |payload| {
|
||||
panic_hook(
|
||||
client.clone(),
|
||||
payload.payload_as_str().unwrap_or("Box<Any>"),
|
||||
payload.location(),
|
||||
)
|
||||
})
|
||||
.expect("failed to attach signal handler");
|
||||
});
|
||||
info!("panic handler registered");
|
||||
let handler = CrashHandler::attach(unsafe {
|
||||
let client = client.clone();
|
||||
let handler = move |crash_context: &crash_handler::CrashContext| {
|
||||
// set when the first minidump request is made to avoid generating duplicate crash reports
|
||||
static REQUESTED_MINIDUMP: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
info!("crash signal handlers installed");
|
||||
send_crash_server_message(&client, CrashServerMessage::Init(crash_init));
|
||||
// only request a minidump once
|
||||
let res = if REQUESTED_MINIDUMP
|
||||
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
|
||||
.is_ok()
|
||||
{
|
||||
#[cfg(target_os = "macos")]
|
||||
macos::suspend_all_other_threads();
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
handler.set_ptracer(Some(_crash_handler.id()));
|
||||
|
||||
info!("crash handler registered");
|
||||
spawn(Box::pin({
|
||||
let client = client.clone();
|
||||
async move {
|
||||
let _handler = { handler };
|
||||
loop {
|
||||
if let Err(e) = client.ping() {
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
log::error!(
|
||||
"ping failed: {:?}, process exit status: {:?}",
|
||||
e,
|
||||
_crash_handler.try_status()
|
||||
);
|
||||
#[cfg(target_os = "windows")]
|
||||
log::error!("ping failed: {:?}", e,);
|
||||
break;
|
||||
};
|
||||
wait_timer(Duration::from_secs(10)).await;
|
||||
// on macos this "ping" is needed to ensure that all our
|
||||
// `client.send_message` calls have been processed before we trigger the
|
||||
// minidump request.
|
||||
client.ping().ok();
|
||||
let r = client.request_dump(crash_context);
|
||||
if let Err(e) = &r {
|
||||
eprintln!("failed to request dump: {:?}", e);
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
macos::resume_all_other_threads();
|
||||
r.is_ok()
|
||||
} else {
|
||||
true
|
||||
};
|
||||
CrashEventResult::Handled(res)
|
||||
};
|
||||
crash_handler::make_crash_event(handler)
|
||||
})
|
||||
.expect("failed to attach signal handler");
|
||||
|
||||
info!("crash signal handlers installed");
|
||||
send_crash_server_message(&client, CrashServerMessage::Init(crash_init));
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
handler.set_ptracer(Some(_crash_handler.id()));
|
||||
|
||||
info!("crash handler registered");
|
||||
spawn(Box::pin({
|
||||
let client = client.clone();
|
||||
async move {
|
||||
let _handler = { handler };
|
||||
loop {
|
||||
if let Err(e) = client.ping() {
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
log::error!(
|
||||
"ping failed: {:?}, process exit status: {:?}",
|
||||
e,
|
||||
_crash_handler.try_status()
|
||||
);
|
||||
#[cfg(target_os = "windows")]
|
||||
log::error!("ping failed: {:?}", e,);
|
||||
break;
|
||||
};
|
||||
wait_timer(Duration::from_secs(10)).await;
|
||||
}
|
||||
}));
|
||||
client
|
||||
}
|
||||
}
|
||||
}));
|
||||
client
|
||||
}
|
||||
|
||||
pub struct CrashServer {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue