Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Fix Console Ctrl-C terminal breaking issue#61740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Console] Fix Console Ctrl-C terminal breaking issue#61740
Uh oh!
There was an error while loading.Please reload this page.
Conversation
- Add signal handlers to restore terminal settings when Ctrl-C is pressed during choice selection- Wrap autocomplete method in try-finally block to ensure terminal restoration- Fixes issue where terminal would be left in raw mode when signal handlers call exit()-Resolvessymfony#61732The issue occurred when using QuestionHelper with ChoiceQuestion and a signal handlerthat calls exit(). The terminal would be left in raw mode (-icanon -echo) becausethe stty restoration only happened at the end of the autocomplete method.This fix:1. Registers signal handlers for common termination signals (SIGINT, SIGQUIT, SIGTERM, etc.)2. Uses try-finally to ensure terminal restoration even if exceptions occur3. Maintains backward compatibility with existing codeReproducer:https://github.com/johnstevenson/sigbugRelated issues:symfony#44045,symfony#48205,symfony#57241
carsonbot commentedSep 12, 2025
Hey! Thanks for your PR. You are targeting branch "7.4" but it seems your PR description refers to branch "7.3". Cheers! Carsonbot |
| // Always restore terminal settings, even if an exception occurs | ||
| $this->restoreTerminal($sttyMode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This would at the very least need to restore previous signal handlers.. But it also needs to call the previous signal handlers when a signal triggers for full compatibility IMO. So this is definitely not a complete fix as it is.
johnstevenson commentedSep 15, 2025
Strange, but this doesn't fix anything when I use your code in the reproducer. |
johnstevenson commentedSep 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ah, you clearly haven't tested this. The reason it doesn't work is that the signal handlers are never set. You need to fix this so that
The issue also occurs in a restarted process as well. While you have ensured that the terminal continues to work, you have unfortunately replaced any user signal handler with your own, which doesn't call exit, thus defeating the most common use of Ctrl-C. You've also wiped out the user signal handler for the remainder of the process after a choice question. I've updated thereproducer so that the questions are called twice, which makes it more obvious where Ctrl-C has been called. I think you need to do the following:
|
nicolas-grekas commentedSep 15, 2025
On review, I wondered if this implementation, if made to work, won't collide with signal handlers as managed by the application? |
johnstevenson commentedSep 15, 2025
Provided that existing signal handlers are saved before the choice question, invoked as I described above if signalled in the choice question, then restored afterwards, then there shouldn't be any collisions. Well that's the theory, anyway. |
mudassaralichouhan commentedSep 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
You can verify the fix by running this command, which simulates a Ctrl-C during the interactive prompt and checks the terminal state before and after: orig=$(stty -g); php sigbug handler& pid=$!; sleep 1;kill -INT$pid;wait$pid;echo"-- stty after --"; stty -a; stty$orig;echo"-- restored --"; stty -a This test was performed on macOS with an M1 chip. Please let me know if you need assistance testing on other platforms or with the restart scenario. |
johnstevenson commentedSep 16, 2025
Thanks for the fixes. However, registering lots of shutdown functions is not ideal or even necessary. I think you are making things a bit too complicated and could use something like this to set the SIGINT handler: $signalHandler =null;if (\function_exists('pcntl_async_signals') &&\function_exists('pcntl_signal')) {pcntl_async_signals(true);$signalHandler =pcntl_signal_get_handler(\SIGINT);pcntl_signal(\SIGINT,function ($signo)use ($sttyMode,$signalHandler) {echo'^C', \PHP_EOL;shell_exec('stty'.$sttyMode);// Original handler will be a callable, SIG_DFL or SIG_IGNif (\is_callable($signalHandler)) {$signalHandler($signo);return; }if ($signalHandler === \SIG_DFL) {exit(128 +$signo); } });} Then call the following (either before throwing the exception and at the end of the function, or just once in a try, finally block); // Restore terminal and original SIGINT handlershell_exec('stty'.$sttyMode);if ($signalHandler !==null) {pcntl_signal(\SIGINT,$signalHandler);} |
johnstevenson commentedSep 16, 2025
This command does not work for me because it shows the same stty state with the broken code. In my terminal the signal stops the process, then restores the terminal so it can tell me it has done so and allow me to resume with |
nicolas-grekas commentedOct 1, 2025
Closing in favor of#61861, thanks for giving this a try! |
Uh oh!
There was an error while loading.Please reload this page.
The issue occurred when using
QuestionHelperwithChoiceQuestionand a signal handler that callsexit(). The terminal would be left in raw mode (-icanon -echo) because thesttyrestoration only happened at the end of theautocomplete()method.This fix:
SIGINT,SIGQUIT,SIGTERM, etc.)try-finallyblock to ensure terminal restoration even if exceptions occur or the process exits earlyregisterTerminalRestoreHandler()— sets up signal-safe terminal restorationrestoreTerminal()— restores the saved terminal mode viasttyReproducer:https://github.com/johnstevenson/sigbug
Related issues:#44045,#48205,#57241