| Message ID | 20200214105810.14588-1-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | 2960107a22b32f6e17794f5e56db718ab82c896f |
| Headers | show |
| Series | sandbox: also restore terminal settings when killed by SIGINT | expand |
On Fri, 14 Feb 2020 at 03:58, Rasmus Villemoes <rasmus.villemoes at prevas.dk> wrote: > > Hitting Ctrl-C is a documented way to exit the sandbox, but it is not > actually equivalent to the reset command. The latter, since it follows > normal process exit, takes care to reset terminal settings and > restoring the O_NONBLOCK behaviour of stdin (and, in a terminal, that > is usually the same file description as stdout and stderr, i.e. some > /dev/pts/NN). > > Failure to restore (remove) O_NONBLOCK from stdout/stderr can cause > very surprising and hard to debug problems back in the terminal. For > example, I had "make -j8" consistently failing without much > information about just exactly what went wrong, but sometimes I did > get a "echo: write error". I was at first afraid my disk was getting > bad, but then a simple "dmesg" _also_ failed with write error - so it > was writing to the terminal that was buggered. And both "make -j8" and > dmesg in another terminal window worked just fine. > > So install a SIGINT handler so that if the chosen terminal > mode (cooked or raw-with-sigs) means Ctrl-C sends a SIGINT, we will > still call os_fd_restore(), then reraise the signal and die as usual > from SIGINT. > > Before: > > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > $ ./u-boot > # hit Ctrl-C > $ grep flags /proc/$$/fdinfo/1 > flags: 0106002 > > After: > > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > $ ./u-boot > # hit Ctrl-C > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> > --- > arch/sandbox/cpu/os.c | 9 +++++++++ > 1 file changed, 9 insertions(+) Nice explanation, thank you. Reviewed-by: Simon Glass <sjg at chromium.org>
On Fri, 14 Feb 2020 at 03:58, Rasmus Villemoes <rasmus.villemoes at prevas.dk> wrote: > > Hitting Ctrl-C is a documented way to exit the sandbox, but it is not > actually equivalent to the reset command. The latter, since it follows > normal process exit, takes care to reset terminal settings and > restoring the O_NONBLOCK behaviour of stdin (and, in a terminal, that > is usually the same file description as stdout and stderr, i.e. some > /dev/pts/NN). > > Failure to restore (remove) O_NONBLOCK from stdout/stderr can cause > very surprising and hard to debug problems back in the terminal. For > example, I had "make -j8" consistently failing without much > information about just exactly what went wrong, but sometimes I did > get a "echo: write error". I was at first afraid my disk was getting > bad, but then a simple "dmesg" _also_ failed with write error - so it > was writing to the terminal that was buggered. And both "make -j8" and > dmesg in another terminal window worked just fine. > > So install a SIGINT handler so that if the chosen terminal > mode (cooked or raw-with-sigs) means Ctrl-C sends a SIGINT, we will > still call os_fd_restore(), then reraise the signal and die as usual > from SIGINT. > > Before: > > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > $ ./u-boot > # hit Ctrl-C > $ grep flags /proc/$$/fdinfo/1 > flags: 0106002 > > After: > > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > $ ./u-boot > # hit Ctrl-C > $ grep flags /proc/$$/fdinfo/1 > flags: 0102002 > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> > --- > arch/sandbox/cpu/os.c | 9 +++++++++ > 1 file changed, 9 insertions(+) Nice explanation, thank you. Reviewed-by: Simon Glass <sjg at chromium.org> Applied to u-boot-dm, thanks!
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index f7c73e3a0b..e7ec892bdf 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -8,6 +8,7 @@ #include <fcntl.h> #include <getopt.h> #include <setjmp.h> +#include <signal.h> #include <stdio.h> #include <stdint.h> #include <stdlib.h> @@ -175,6 +176,13 @@ void os_fd_restore(void) } } +static void os_sigint_handler(int sig) +{ + os_fd_restore(); + signal(SIGINT, SIG_DFL); + raise(SIGINT); +} + /* Put tty into raw mode so <tab> and <ctrl+c> work */ void os_tty_raw(int fd, bool allow_sigs) { @@ -205,6 +213,7 @@ void os_tty_raw(int fd, bool allow_sigs) term_setup = true; atexit(os_fd_restore); + signal(SIGINT, os_sigint_handler); } void *os_malloc(size_t length)
Hitting Ctrl-C is a documented way to exit the sandbox, but it is not actually equivalent to the reset command. The latter, since it follows normal process exit, takes care to reset terminal settings and restoring the O_NONBLOCK behaviour of stdin (and, in a terminal, that is usually the same file description as stdout and stderr, i.e. some /dev/pts/NN). Failure to restore (remove) O_NONBLOCK from stdout/stderr can cause very surprising and hard to debug problems back in the terminal. For example, I had "make -j8" consistently failing without much information about just exactly what went wrong, but sometimes I did get a "echo: write error". I was at first afraid my disk was getting bad, but then a simple "dmesg" _also_ failed with write error - so it was writing to the terminal that was buggered. And both "make -j8" and dmesg in another terminal window worked just fine. So install a SIGINT handler so that if the chosen terminal mode (cooked or raw-with-sigs) means Ctrl-C sends a SIGINT, we will still call os_fd_restore(), then reraise the signal and die as usual from SIGINT. Before: $ grep flags /proc/$$/fdinfo/1 flags: 0102002 $ ./u-boot # hit Ctrl-C $ grep flags /proc/$$/fdinfo/1 flags: 0106002 After: $ grep flags /proc/$$/fdinfo/1 flags: 0102002 $ ./u-boot # hit Ctrl-C $ grep flags /proc/$$/fdinfo/1 flags: 0102002 Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> --- arch/sandbox/cpu/os.c | 9 +++++++++ 1 file changed, 9 insertions(+)