Skip to content

crashes: Avoid crash handler on detached threads#40883

Merged
nia-e merged 1 commit intomainfrom
extension-crash-fix
Oct 23, 2025
Merged

crashes: Avoid crash handler on detached threads#40883
nia-e merged 1 commit intomainfrom
extension-crash-fix

Conversation

@nia-e
Copy link
Contributor

@nia-e nia-e commented Oct 22, 2025

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

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 22, 2025
@dinocosta dinocosta force-pushed the extension-crash-fix branch from ddc89d0 to ba3b675 Compare October 22, 2025 11:49
@P1n3appl3
Copy link
Contributor

P1n3appl3 commented Oct 22, 2025

I think this ignores too many panics. Generally we still want panics in detached threads to crash the application, a quick "goto refs" indicates that there are around 450 thread detaches in our codebase, and they're used for all sorts of things. If we think the background thread running extensions specifically should be allowed to panic, I'd recommend to special case it with the thread local (or even use catch_panic for it tbh, given that we compile with unwinding support).

@nia-e nia-e force-pushed the extension-crash-fix branch from ba3b675 to 8327345 Compare October 23, 2025 20:33
@nia-e
Copy link
Contributor Author

nia-e commented Oct 23, 2025

I'll leave the formatting nits in gpui/executor.rs but this should be good!

@nia-e nia-e enabled auto-merge (squash) October 23, 2025 20:34
@nia-e nia-e force-pushed the extension-crash-fix branch from 8327345 to 0074194 Compare October 23, 2025 20:41
@nia-e nia-e merged commit 68707ff into main Oct 23, 2025
22 checks passed
@nia-e nia-e deleted the extension-crash-fix branch October 23, 2025 21:04
Veykril added a commit that referenced this pull request Nov 18, 2025
)

#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
github-actions bot pushed a commit that referenced this pull request Nov 18, 2025
)

#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
github-actions bot pushed a commit that referenced this pull request Nov 18, 2025
)

#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
Veykril added a commit that referenced this pull request Nov 19, 2025
) (cherry-pick to stable) (#43017)

Cherry-pick of #43005 to stable

----
#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

---------

Co-authored-by: Lukas Wirth <lukas@zed.dev>
Veykril added a commit that referenced this pull request Nov 19, 2025
) (cherry-pick to stable) (#43016)

Cherry-pick of #43005 to stable

----
#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

---------

Co-authored-by: Lukas Wirth <lukas@zed.dev>
11happy pushed a commit to 11happy/zed that referenced this pull request Dec 1, 2025
…-industries#43005)

zed-industries#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
@ConradIrwin ConradIrwin mentioned this pull request Feb 24, 2026
3 tasks
ConradIrwin added a commit that referenced this pull request Feb 24, 2026
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.
Anthony-Eid pushed a commit to bobbymannino/zed that referenced this pull request Feb 25, 2026
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 zed-industries#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.
tahayvr pushed a commit to tahayvr/zed that referenced this pull request Mar 4, 2026
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 zed-industries#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants