Linux crash reports for runtime-initiated aborts currently carry no
information about why the runtime aborted. A sizable family of Sentry
issues (FR-108 / [ZED-9SC](https://zed-dev.sentry.io/issues/7581925141/)
among them) consists of glibc's malloc integrity checks detecting heap
corruption and aborting from inside `malloc`/`realloc`/`free`, so each
event lands on a random victim stack and the underlying problem stays
invisible and ungroupable.
glibc records the diagnostic it prints before aborting ("free(): invalid
pointer", "double free or corruption", assertion failures,
stack-smashing reports) in the private `__abort_msg` global specifically
so it can be recovered post-mortem. This resolves that symbol's address
at startup and sends it to the crash-handler process; when a crash comes
in, the handler reads the message out of the crashed process's memory
with `process_vm_readv` while the client is still parked in its signal
handler awaiting the dump acknowledgement. The message is then attached
to crash uploads as a searchable `abort_message` tag plus a full-text
context. The crashed process does no work itself, which matters because
in exactly these crashes the crashed thread may hold the allocator's
arena lock, so it cannot safely allocate or format anything. Everything
degrades gracefully to the current behavior when the symbol is
unavailable (musl, future glibc changes).
With the tag in place, Sentry fingerprint rules can fold these
corruption aborts into a single tracked issue instead of minting a new
single-event issue per victim stack, and the message text distinguishes
double-frees from out-of-bounds metadata corruption when hunting the
actual culprit.
Release Notes:
- N/A
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
Self-Review Checklist:
- [ ] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [ ] Tests cover the new/changed behavior
- [ ] Performance impact has been considered and is acceptable
Closes #ISSUE
Release Notes:
- N/A or Added/Fixed/Improved ...
String panics are a non-trivial percentage of the crashes we see at Zed,
and
doubly unfortunately they may incidentally include the contents of a
user's
buffer.
Although this hasn't happened yet (to my knowledge), I don't want to be
in the position of having received sensitive information this way.
See also https://github.com/rust-lang/rust/pull/153677
Release Notes:
- N/A
Before this change the crash handler uploaded crashes before sign-in had
happened.
Now we get the metrics_id correctly. This allows for us to tie crashes
reported
on Github to users who have opted into telemetry (users who have opted
into
crash reporting but not telemetry will not have a metrics_id).
Release Notes:
- Fixed crash reporter metadata collection
---------
Co-authored-by: Miguel Raz Guzmán Macedo <miguel@zed.dev>
#50176 added `recoverable_panic` to swallow mermaid panics, then #50280
fixed it in upstream. This PR removes the workaround so future panics
reach Sentry. Also bumps `mermaid-rs-renderer` to fix a hex parsing
panic.
Release Notes:
- N/A
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.
We see a number of crashes in Sentry that appear to be crashes in
wasmtime.
This shouldn't happen, as wasmtime is designed to run untrusted code
"safely".
Looking into this, it seems likely that the problem is that we race with
wasmtime
when installing signal handlers. If wasmtime's handlers are installed
before ours,
then any signals that it intends to handle (like out of bounds memory
access) will
reach our handlers before its; which causes us to assume the app has
crashed.
This changes fixes our crash handler initialization to ensure we always
create
our signal handler first, and reverts a previous attempt to fix this
from #40883
Closes #ISSUE
Before you mark this PR as ready for review, make sure that you have:
- [ ] Added a solid test coverage and/or screenshots from doing manual
testing
- [ ] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
Release Notes:
- Linux: Fixed crashes that could happen due to our crash handler
erroneously catching signals intended for wasmtime.
https://github.com/zed-industries/zed/pull/40883 implemented this
incorrectly. It was marking a random background thread as a wasm thread
(whatever thread picked up the wasm epoch timer background task),
instead of marking the threads that actually run the wasm extension.
This has two implications:
1. it didn't prevent extension panics from tearing down as planned
2. Worse, it actually made us hide legit panics in sentry for one of our
background workers.
Now 2 still technically applies for all tokio threads after this, but we
basically only use these for wasm extensions in the main zed binary.
Release Notes:
- Fixed extension panics crashing Zed on Linux
Closes#41471
We were killing the crash handler when it received a second copy of any
of the messages, but this GPU specs one is sent on each new window
rather than once at startup. We could gate the sending to only happen
once, but it's simpler to just allow multiple gpu specs messages.
Release Notes:
- N/A
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 <dinojoaocosta@gmail.com>
This allows to filter by main zed binary or remote server crashes, as
well as easily tell whether a crash happened in a remote-server binary
or not.
Release Notes:
- N/A
std commands can block for an arbitrary duration and so runs risk of
blocking tasks for too long. This replaces all such uses where sensible
with async processes.
Release Notes:
- N/A *or* Added/Fixed/Improved ...
@maxdeviant We can eventually turn down the panic telemetry endpoint,
but should probably leave it up while there's still a bunch of stable
users hitting it.
@maxbrunsfeld We're optimistic that this change also fixed the macos
crashed-thread misreporting. We think it was because the
`CrashContext::exception` was getting set to `None` only on macos, while
on linux it was getting a real exception value from the sigtrap. Now
we've unified and it uses `SIGABRT` on both platforms (I need to double
check that this works as expected for windows).
We unconditionally set `RUST_BACKTRACE=1` for the current process so
that we see backtraces when running in a terminal by default. This
should be fine but I just wanted to note it since it's a bit abnormal.
Release Notes:
- N/A
---------
Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Closes #ISSUE
Adds system GPU collection to crash reporting. Currently this is Linux
only.
The system GPUs are determined by reading the `/sys/class/drm` directory
structure, rather than using the exisiting `gpui::Window::gpu_specs()`
method in order to gather more information, and so that the GPU context
is not dependent on Vulkan context initialization (i.e. we still get GPU
info when Zed fails to start because Vulkan failed to initialize).
Unfortunately, the `blade` APIs do not support querying which GPU _will_
be used, so we do not know which GPU was attempted to be used when
Vulkan context initialization fails, however, when Vulkan initialization
succeeds, we send a message to the crash handler containing the result
of `gpui::Window::gpu_specs()` to include the "Active" gpu in any crash
report that may occur
Release Notes:
- N/A *or* Added/Fixed/Improved ...
On the latest build @maxbrunsfeld got a panic that hung zed. It appeared
that the hang occured after the minidump had been successfully written,
so our theory on what happened is that the `suspend_all_other_threads`
call in the crash handler suspended the panicking thread (due to the
signal from simulate_exception being received on a different thread),
and then when the crash handler returned everything was suspended so the
panic hook never made it to the `process::abort`.
This change makes the crash handler avoid _both_ the current and the
panicking thread which should avoid that scenario.
Release Notes:
- N/A
We see a bunch of crash events with truncated minidumps where they have
a valid header but no events. We think this is due to an issue
generating them, so we're attaching the relevant result to the uploaded
tags.
Release Notes:
- N/A
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
This should improve our detection of which thread crashed since they
wont be able to resume while the minidump is being generated.
Release Notes:
- N/A
The minidump-based crash reporting is now entirely separate from our
legacy panic_hook-based reporting. This should improve the association
of minidumps with their metadata and give us more consistent crash
reports.
Release Notes:
- N/A
---------
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
We only want minidumps to be generated on actual release builds. Now we
avoid spawning crash handler processes for dev builds. To test
minidumping you can still set the `ZED_GENERATE_MINIDUMPS` env var which
force-enable the feature.
Release Notes:
- N/A
- [x] Handle uploading minidumps from the remote_server
- [x] Associate minidumps with panics with some sort of ID (we don't use
session_id on the remote)
- [x] Update the protobufs and client/server code to request panics
- [x] Upload minidumps with no corresponding panic
- [x] Fill in panic info when there _is_ a corresponding panic
- [x] Use an env var for the sentry endpoint instead of hardcoding it
Release Notes:
- Zed now generates minidumps for crash reporting
---------
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
Richard Feldman