terminal: Close the terminal if EOF is detected by lemorage · Pull Request #39082 · zed-industries/zed

lemorage · 2025-09-29T04:17:50Z

Release Notes:

Fixed terminal freezing on Linux when pressing Ctrl+C followed by Ctrl+D or typing exit (Terminal freezes in Linux session when Ctrl+C is pressed before exit #38901)

Implementation Details:

Interactive shells now only close when user has sent keyboard input
Spawn failures (which never receive keyboard input) keep the terminal open to show error messages
User-initiated exits via Ctrl+D or typing "exit" close the terminal as expected
Tasks/commands (Shell::WithArguments) continue to close based on exit code (0 = success)

SomeoneToIgnore · 2025-09-29T10:07:35Z

Passing by and noting that this change is a bit odd and things were working in Linux for me before.
Could be that recent Alacritty bump could have caused this, and we need to understand what had changed and was this a regression or a new configuration is needed?

lemorage · 2025-09-29T10:58:50Z

Umm, possible. I never tested the before version in linux. I can grab some time see if any Alacritty related issue resulting in this.

Veykril · 2025-09-29T11:14:52Z

I'd like for us to see if we can make a simple test for this behavior as it feels like its easy for us to break things here (as is evident by this PR being raised now). Would you mind having a brief look at whether this is easily testable? If not I'll try to commit a bit of time into looking into that myself this week.

lemorage · 2025-09-29T11:20:28Z

Sure. Np!

SomeoneToIgnore · 2025-09-30T19:46:11Z

#39232 adds a test that fails on Linux, to help with fixing the issue.

lemorage · 2025-10-01T06:41:55Z

Thanks! I'll see!

Veykril · 2025-10-02T07:21:49Z

crates/terminal/src/terminal.rs

+                        /* Close only if success or no exit code */
+                        self.child_exited.is_none_or(|e| e.code() == Some(0))
+                    } else {
+                        /* Always close the interactive shell */


I think this will cause us to close the shell again if we fail to spawn it for whatever reason. Now, that shouldn't really be happening but an example would be a $SHELL being misconfigured and us using Shell::System, which we then again don't surface to the user because of the immediate closing of the shell

Valid concern. But tbh, a fundamental problem is that we cannot distinguish between:

PTY Exit from spawn failure

PTY Exit from user pressing Ctrl+D
So I find it hard to have the best of both worlds. Do u have any insights on this?

Hmm, that is problematic. Is there really no way for us to differentiate spawn failure from a EOF signal here? ...

Veykril · 2025-10-21T15:10:28Z

crates/terminal/src/terminal.rs

+                    let is_interactive_shell =
+                        !matches!(self.template.shell, Shell::WithArguments { .. });


This doesn't work. Either of the three variants could be an interactive shell. A better approach i think would be to check if self.task.is_none(), then we are definitely a shell session and not a task

Good idea! I'll see.

Veykril · 2025-10-28T07:11:43Z

crates/terminal/src/terminal.rs

+        // Give shell time to process SIGINT
+        smol::Timer::after(Duration::from_millis(50)).await;


This seems a bit odd, we have the event receiving logic from before expecting a wakeup after all. We should keep that instead, if the events differ on platforms we might want to make this a bit more flexible, but waiting for a given duration makes the test flaky

I remembered this test had some issues on Linux. I'll check this later on my Linux machine.

Veykril · 2025-10-28T07:12:59Z

crates/terminal/src/terminal.rs

            Some(task) => task,
            None => {
-                if self.child_exited.is_none_or(|e| e.code() == Some(0)) {
+                // Always record the latest exit status.


Can we add a comment to this entire block describing the intent of this PR? Otherwise this logic looks arbitrary to readers and we might regress unintentionally

Sure thing!

lemorage · 2025-10-29T15:02:59Z

After Ctrl + C and Ctrl + D, the terminal exits smoothly on Linux. This is verified. But in the test_terminal_eof, whatever approaches I try, it always fails. It works only after a short interval here, say smol::Timer::after(Duration::from_millis(50)).await;. So I still keep it there in the test. Feel free to tell if you have any insights.

Veykril

Sorry for the delay, turns out I never submitted this review

Veykril · 2025-10-31T08:10:22Z

crates/terminal/src/terminal.rs

-        let wakeup = event_rx.recv().await.expect("No wakeup event received");
-        assert_eq!(wakeup, first_event, "Expected wakeup, got {wakeup:?}");
+        #[cfg(target_os = "linux")]
+        let first_event = Event::BreadcrumbsChanged;


Given this is weirdly the first event on linux, could you check if the timing issue disappears if we make it poll events until the first wakeup event comes in? I unfortunately do not have a linux machine set up at the moment so I can't test this myself.

lemorage · 2025-11-17T04:10:14Z

I did some time of testing. So if we poll events until the first wakeup event comes in, the test still fails on Linux unless we adds the following:

while let Ok(Ok(_first_event)) = smol_timeout(Duration::from_millis(100), event_rx.recv()).await {}

But this is only a remedy on Linux end, which will in turn cause tests failing on MacOS. So whatever a special case #[cfg(target_os = "linux")] is unavoidable, per my cents. Or I would like to hear if there is anything I am missing.

Veykril · 2026-01-23T10:08:40Z

Sorry for the silence on my part, @smitbarmase will take this over and have a look as they have a linux machine to test this on

lemorage · 2026-01-23T12:42:42Z

@Veykril Thanks! No worries!

smitbarmase · 2026-03-25T07:55:29Z

Hey @lemorage, any findings after rebase?

lemorage · 2026-03-25T10:58:04Z

@smitbarmase Could u make the CI tests running? The tests work properly on my Mac, but I am not sure if they are all passed across platforms.

smitbarmase

Hey, thanks for this. Looking at the code, it seems like a bunch of cases don’t really make sense. Also, from testing, it doesn’t do what it claims to do, since the keyboard_input_sent heuristic is unreachable.

I’m sure this may have drifted during a rebase or changed further on top of the original patch. I’m going to close this for now, but feel free to take another shot at it. I think we should have tests covering every branch this might introduce.

After checking test_terminal_eof more, it is indeed flaky on Linux because we call Ctrl-C and Ctrl-D right after each other. Sometimes Ctrl-C causes consuming/flushing behavior, which makes the test behave inconsistently. This needs special attention to fix, and I think it’s a separate issue from this patch.

A better way to test the behavior we want would be to run false, and then exit. That also blocks the terminal from closing, and it reproduces on both macOS and Linux with bash.

smitbarmase · 2026-03-26T17:12:16Z

crates/terminal/src/terminal.rs

            None => {
-                if self.child_exited.is_none_or(|e| e.code() == Some(0)) {
+                // Always record the latest exit status.
+                if let Some(e) = exit_status {


We already do this on line 2241 right? Why do we need this again right after it?

smitbarmase · 2026-03-26T17:13:10Z

crates/terminal/src/terminal.rs

+                //
+                // For tasks/commands, we close on successful exit (exit code 0).
+                let should_close = if raw_status.is_none() {
+                    if self.task.is_none() {


We already know we are in task is None case, so this is always true?

smitbarmase · 2026-03-26T17:26:18Z

crates/terminal/src/terminal.rs

+                // receive keyboard input, while user exits always require it.
+                //
+                // For tasks/commands, we close on successful exit (exit code 0).
+                let should_close = if raw_status.is_none() {


Also, raw_status is never None (if you check the call sites) so everything inside it is just a dead code.

Closes #38901 Supersedes #39082 This PR fixes an issue where the terminal tab would stay open after the user exits a shell that has a non-zero exit code (e.g. running `false` then `exit`, or pressing Ctrl-C followed by Ctrl-D). We now track whether any keyboard input was sent to distinguish user-initiated exits from shell spawn failures. Release Notes: - Fixed terminal tab not closing when the shell exits with a non-zero code. Co-authored-by: Glenn Miao <one.lemorage@gmail.com>

cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Sep 29, 2025

SomeoneToIgnore assigned Veykril Sep 29, 2025

lemorage force-pushed the fixes branch from 478c350 to d9bb398 Compare October 1, 2025 15:51

Veykril reviewed Oct 2, 2025

View reviewed changes

SomeoneToIgnore mentioned this pull request Oct 17, 2025

terminal: Fix terminal freezing caused by non-zero exit code #40456

Closed

lemorage force-pushed the fixes branch 2 times, most recently from 3667b25 to bb3b4a9 Compare October 21, 2025 14:11

Veykril reviewed Oct 21, 2025

View reviewed changes

lemorage force-pushed the fixes branch from 4fa33c8 to 4e49315 Compare October 22, 2025 14:00

Veykril reviewed Oct 28, 2025

View reviewed changes

lemorage added 5 commits October 28, 2025 21:34

terminal: Fix Linux terminal freeze after SIGINT + EOF

b9507ee

Fix clippy lint issues

d8682de

terminal: Always close interactive shells on PTY exit

24d50a1

Track shell startup to differentiate spawn failures from user exits

809643a

Update test_terminal_eof to respect OS differences

e7ad783

lemorage force-pushed the fixes branch from 4e49315 to e7ad783 Compare October 29, 2025 14:41

Veykril reviewed Nov 13, 2025

View reviewed changes

lemorage added 2 commits November 16, 2025 22:58

Poll any non-wakeup events on Linux

88bb366

Merge branch 'main' into fixes

7f749e9

lemorage force-pushed the fixes branch from e2ffcb2 to 7f749e9 Compare November 17, 2025 02:37

No intermediate processing of events after Ctrl+C

27c3abe

zelenenka added this to Quality Week – December 2025 Nov 27, 2025

github-project-automation bot moved this to Community PRs in Quality Week – December 2025 Nov 27, 2025

lemorage added 2 commits December 5, 2025 23:24

Merge branch 'main' into fixes

ecbb384

Merge branch 'main' into fixes

75d0c3c

lemorage force-pushed the fixes branch from 54064d0 to 191bf5d Compare December 7, 2025 14:13

Poll to avoid unexpected first event on Linux

16d0665

lemorage force-pushed the fixes branch 2 times, most recently from cebcf05 to a2e3281 Compare December 7, 2025 15:05

Fix first event issue on test_terminal_eof

59e9783

lemorage force-pushed the fixes branch from a2e3281 to 59e9783 Compare December 7, 2025 15:20

Veykril assigned smitbarmase Jan 22, 2026

Veykril removed their assignment Jan 23, 2026

Merge branch 'main' into fixes

738a871

This was referenced Jan 27, 2026

Terminal does not close on EOF #47645

Open

Terminal exit issue #48099

Closed

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into fixes

539cad3

github-actions bot added the size/S label Mar 19, 2026

smitbarmase reviewed Mar 26, 2026

View reviewed changes

smitbarmase closed this Mar 26, 2026

github-project-automation bot moved this from Community PRs to Done in Quality Week – December 2025 Mar 26, 2026

smitbarmase mentioned this pull request Mar 26, 2026

terminal: Fix terminal not closing after non-zero shell exit #52520

Merged

		let is_interactive_shell =
		!matches!(self.template.shell, Shell::WithArguments { .. });

		// Give shell time to process SIGINT
		smol::Timer::after(Duration::from_millis(50)).await;

Uh oh!

Conversation

lemorage commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SomeoneToIgnore commented Sep 29, 2025

Uh oh!

lemorage commented Sep 29, 2025

Uh oh!

Veykril commented Sep 29, 2025

Uh oh!

lemorage commented Sep 29, 2025

Uh oh!

SomeoneToIgnore commented Sep 30, 2025

Uh oh!

lemorage commented Oct 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lemorage commented Oct 29, 2025

Uh oh!

Veykril left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lemorage commented Nov 17, 2025

Uh oh!

Veykril commented Jan 23, 2026

Uh oh!

lemorage commented Jan 23, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

smitbarmase commented Mar 25, 2026

Uh oh!

lemorage commented Mar 25, 2026

Uh oh!

smitbarmase left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lemorage commented Sep 29, 2025 •

edited

Loading