Skip to content

Use ExitStatus in ChildExit event#8825

Merged
chrisduerr merged 5 commits intoalacritty:masterfrom
zed-industries:fix-child-exit-patch
Jan 29, 2026
Merged

Use ExitStatus in ChildExit event#8825
chrisduerr merged 5 commits intoalacritty:masterfrom
zed-industries:fix-child-exit-patch

Conversation

@smitbarmase
Copy link
Contributor

@smitbarmase smitbarmase commented Jan 22, 2026

This change passes ExitStatus directly in ChildEvent::Exited and Event::ChildExit instead of i32.

Previously, ExitStatus::code() was used which returns None for signal terminations, so downstream couldn't detect when a process was killed by a signal. Now downstream receives the full ExitStatus and can use .code(), .signal(), etc. as needed.

Downstream PR: zed-industries/zed#47420

smitbarmase added a commit to zed-industries/zed that referenced this pull request Jan 25, 2026
…47420)

Closes #38168
Closes #42414

We were already using `ExitStatusExt::from_raw()` to construct an
`ExitStatus`, but we were passing in the exit code from
alacritty_terminal's `ChildExit` event. This worked fine on Windows
where `from_raw()` expects an exit code, but on Unix, `from_raw()`
([read
more](https://doc.rust-lang.org/std/os/unix/process/trait.ExitStatusExt.html#tymethod.from_raw))
expects a raw wait status from `waitpid()` and not an exit code.

When a child process was killed by a signal (e.g., SIGSEGV),
`ExitStatus::code()` returns `None` since only normal exits have an exit
code. This caused the terminal to hang because we weren't properly
detecting the exit.

One fix would have been to remove the dependency on `ExitStatus`
entirely, but using the raw wait status gives us more information, we
can now detect exit codes, signal terminations, and more.

The actual fix was upstream in `alacritty_terminal` to send the raw wait
status instead of just the exit code.

Currently using forked patch
https://github.com/zed-industries/alacritty/tree/v0.16-child-exit-patch
which is based on v0.25.1.

Upstream PR: alacritty/alacritty#8825

Release Notes:

- Fixed terminal hanging when a child process is killed by a signal
(e.g., SIGSEGV from null pointer dereference).
Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment.

@smitbarmase smitbarmase changed the title Use raw wait status instead of exit code in ChildExit event Use ExitStatus in ChildExit event Jan 28, 2026
@smitbarmase
Copy link
Contributor Author

Tested on Windows and macOS, works well with the downstream changes. Ready for review when you get a chance.

Comment on lines +83 to +84
/// Indicates the child has exited.
Exited(Option<ExitStatus>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as an observation: Windows is the only reason why this needs to be an Option at this point. I'd be surprised if this was ever actually None in practice.

Event::Bell => write!(f, "Bell"),
Event::Exit => write!(f, "Exit"),
Event::ChildExit(code) => write!(f, "ChildExit({code})"),
Event::ChildExit(status) => write!(f, "ChildExit({status:?})"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what this formats as: ChildExit(ExitStatus(unix_wait_status(120))). I think for a debug format that's probably fine since the STD internals are pretty clear here.

@chrisduerr chrisduerr merged commit 4225cea into alacritty:master Jan 29, 2026
5 checks passed
@smitbarmase smitbarmase deleted the fix-child-exit-patch branch February 10, 2026 13:41
smitbarmase added a commit to zed-industries/zed that referenced this pull request Feb 10, 2026
Follow-up to #47420 and #44193.

#47420 fixed child-exit status handling via upstream alacritty change
(alacritty/alacritty#8825). However,
stable/preview/nightly builds still reproduced hangs that dev builds did
not.

The root cause matches #44193: spawned children inherit crash-handler
Mach exception ports and hang. We already reset exception ports in
`util::command::new_smol_command` and
`util::set_pre_exec_to_start_new_session`, but PTY child spawning in
`alacritty_terminal` was not covered.

This PR updates `alacritty_terminal` to include PTY `pre_exec`
exception-port reset:

zed-industries/alacritty@9d9640d

Upstream:
- alacritty/alacritty#8835

Release Notes:

- Fixed terminal tasks hanging on macOS when a spawned process is killed
by a signal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants