Skip to content

Conversation

zeld-a
Copy link
Contributor

@zeld-a zeld-a commented Oct 17, 2025

Closes #38901 and #40283

Release Notes:

  • Fixed bug causing the terminal pane to freeze upon invoking exit or Ctrl+D when the previous command had a non-zero exit code

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 17, 2025
@SomeoneToIgnore
Copy link
Contributor

For the context, we have another attempt here: #39082

@SomeoneToIgnore
Copy link
Contributor

Another context bit.
We have a nice test for this, that should be running on Linux too after whatever fix is merged:

// TODO should be tested on Linux too, but does not work there well
#[cfg(target_os = "macos")]
#[gpui::test(iterations = 10)]

@zeld-a zeld-a changed the title Terminal: Fixed Terminal freezing caused by SIGINT. Terminal: Fixed Terminal freezing caused by SIGINT Oct 17, 2025
@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 17, 2025

@SomeoneToIgnore should I worry about the failed test? it seems to be a worktree issue but i did not touch any of that logic.

@SomeoneToIgnore
Copy link
Contributor

Looks like an unfortunate flak to me.
I worry much more about 0 tests this PR brings with a fix.

@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 17, 2025

would you like me to add a test?

@SomeoneToIgnore
Copy link
Contributor

#40456 (comment)

@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 17, 2025

Sorry, missed that message. Thx.

@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 17, 2025

Concerning the test, here are my findings...

  1. The first_event is not always Wakeup. Sometimes it is BreadcrumbsChanged. Since this is not what we are testing, a simple statement to ensure the terminal is created and ready to go should suffice.
  2. The current implementation of passing keystrokes to the terminal does not seem to do that on Linux. The only logical explanation for this that I can come up with is that the psuedo-terminal being created is non canonical, meaning it will not accept key binds.
  3. Passing bytes seems to work using terminal.input(b"exit\n"), however if you pass \x03(Ctrl-C) or \x04(Ctrl-D), it is sent to the shell which results in the terminal exiting with exit codes 130 and 0 respectively.

I am going to continue trying to find a way around the inability to invoke SIGINT but if anyone has some suggestions, feel free to let me know.

@maxdeviant maxdeviant changed the title Terminal: Fixed Terminal freezing caused by SIGINT terminal: Fix terminal freezing caused by SIGINT Oct 17, 2025
@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 18, 2025

Please disregard my previous comment. It was misinformed on certain aspects.

I have deduced that the exit code of the process before EOF is the exit code that will be returned. If it is Ctrl-C it returns 130, ls nonexistent/directory returns 2, false returns 1, etc.

The nature of my fix for the freezing bug was to just allow the 130 and the 0 to be valid exit codes. Instead of testing for exit code 0, the test now simply tests to see if it exits at all after the keystrokes are passed. As long as we receive an exit code after EOF the test will pass.

@zeld-a zeld-a changed the title terminal: Fix terminal freezing caused by SIGINT terminal: Fix terminal freezing caused by non-zero exit code Oct 18, 2025
@zeld-a
Copy link
Contributor Author

zeld-a commented Oct 18, 2025

@Veykril I have tried my hand at fixing this issue, but the issue is steming from ExitStatusExt::from_raw(code) returning the exit code of the process before EOF. This is specific to linux and i haven't the slightest inkling why it does this. I am closing this as the current fix does not pass all tests,

@zeld-a zeld-a closed this Oct 18, 2025
@zeld-a zeld-a deleted the bug/SIGINT-terminal_freeze branch October 18, 2025 14:10
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.

Terminal freezes in Linux session when Ctrl+C is pressed before exit

3 participants