From f87590fe9ee1746e299710ee2e36b3132a311188 Mon Sep 17 00:00:00 2001 From: Andy C Date: Thu, 27 Jan 2022 11:36:42 -0500 Subject: [PATCH] [signal-handling] Ignore untrapped SIGWINCH on wait. This fixes #1067 for real this time! I use a slightly odd technique of setting SignalState::last_sig_num to the sentinel UNTRAPPED_SIGWINCH. We still need to redesign signal handling for the C++ translation. Add a failing test for the analogous 'wait -n' bug, which still needs to be fixed. It needs to run in a loop until a state is reached. --- core/process.py | 1 + core/pyos.py | 14 +++++++++++--- osh/builtin_process.py | 20 +++++++++++++++----- test/interactive.py | 18 ++++++++++++++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/core/process.py b/core/process.py index 3b853c3905..8c42fff12d 100644 --- a/core/process.py +++ b/core/process.py @@ -1435,6 +1435,7 @@ def WaitForOne(self): if e.errno == ECHILD: return W1_ECHILD # nothing to wait for caller should stop elif e.errno == EINTR: # Bug #858 fix + #log('WaitForOne() => %d', self.sig_state.last_sig_num) return self.sig_state.last_sig_num # e.g. 1 for SIGHUP else: # The signature of waitpid() means this shouldn't happen diff --git a/core/pyos.py b/core/pyos.py index e3282f0bf0..470ef4565b 100644 --- a/core/pyos.py +++ b/core/pyos.py @@ -14,6 +14,7 @@ import time from core import pyutil +from core.pyerror import log import posix_ as posix @@ -241,18 +242,25 @@ def InputAvailable(fd): return len(r) != 0 +UNTRAPPED_SIGWINCH = -1 + class SigwinchHandler(object): """Wrapper to call user handler.""" - def __init__(self, display): - # type: (_IDisplay) -> None + def __init__(self, display, sig_state): + # type: (_IDisplay, SignalState) -> None self.display = display + self.sig_state = sig_state self.user_handler = None # type: _TrapHandler def __call__(self, sig_num, unused_frame): # type: (int, Any) -> None """For Python's signal module.""" + # SENTINEL for UNTRAPPED SIGWINCH. If it's trapped, self.user_handler + # will overwrite it with signal.SIGWINCH. + self.sig_state.last_sig_num = UNTRAPPED_SIGWINCH + self.display.OnWindowChange() if self.user_handler: self.user_handler(sig_num, unused_frame) @@ -301,7 +309,7 @@ def InitInteractiveShell(self, display): # This is ALWAYS on, which means that it can cause EINTR, and wait() and # read() have to handle it - self.sigwinch_handler = SigwinchHandler(display) + self.sigwinch_handler = SigwinchHandler(display, self) signal.signal(signal.SIGWINCH, self.sigwinch_handler) def AddUserTrap(self, sig_num, handler): diff --git a/osh/builtin_process.py b/osh/builtin_process.py index 6ac816abb7..c0d897c248 100644 --- a/osh/builtin_process.py +++ b/osh/builtin_process.py @@ -23,6 +23,7 @@ from core import main_loop from core.pyutil import stderr_line from core import process # W1_OK, W1_ECHILD +from core import pyos from core import vm from core.pyerror import log from frontend import args @@ -39,7 +40,6 @@ if TYPE_CHECKING: from _devbuild.gen.syntax_asdl import command_t from core.process import ExternalProgram, FdState, JobState, Waiter - from core.pyos import SignalState from core.state import Mem, SearchPath from core.ui import ErrorFormatter from frontend.parse_lib import ParseContext @@ -171,7 +171,7 @@ def _Run(self, cmd_val): if result == process.W1_ECHILD: # nothing to wait for, or interrupted. status is 0 break - elif result >= 0: # signal + elif result >= 0 and result != pyos.UNTRAPPED_SIGWINCH: # signal status = 128 + result break @@ -289,10 +289,20 @@ def Run(self, cmd_val): class _TrapHandler(object): """A function that is called by Python's signal module. - Similar to process.SubProgramThunk.""" + Similar to process.SubProgramThunk. + + TODO: In C++ we can't use this type of handling. We cannot append to a + garbage-colleted list inside a signal handler! + + Instead I think we need to append to a global array of size 1024 for the last + signal number caught. + + Then in the main loop we will have RunPendingTraps() that iterates over this + list, runs corresponding handlers, and then clears the list. + """ def __init__(self, node, nodes_to_run, sig_state, tracer): - # type: (command_t, List[command_t], SignalState, dev.Tracer) -> None + # type: (command_t, List[command_t], pyos.SignalState, dev.Tracer) -> None self.node = node self.nodes_to_run = nodes_to_run self.sig_state = sig_state @@ -345,7 +355,7 @@ def _GetSignalNumber(sig_spec): class Trap(vm._Builtin): def __init__(self, sig_state, traps, nodes_to_run, parse_ctx, tracer, errfmt): - # type: (SignalState, Dict[str, _TrapHandler], List[command_t], ParseContext, dev.Tracer, ErrorFormatter) -> None + # type: (pyos.SignalState, Dict[str, _TrapHandler], List[command_t], ParseContext, dev.Tracer, ErrorFormatter) -> None self.sig_state = sig_state self.traps = traps self.nodes_to_run = nodes_to_run diff --git a/test/interactive.py b/test/interactive.py index 5bb474ce3f..6581ef8235 100755 --- a/test/interactive.py +++ b/test/interactive.py @@ -197,6 +197,24 @@ def sigwinch_untrapped_wait(sh): sh.expect('status=0') +@register() +def sigwinch_untrapped_wait_n(sh): + 'untrapped SIGWINCH during wait -n' + + sh.sendline('sleep 1 &') + sh.sendline('wait -n') + + time.sleep(0.1) + + # simulate window size change + sh.kill(signal.SIGWINCH) + + sh.expect(r'.*\$') # expect prompt + + sh.sendline('echo status=$?') + sh.expect('status=0') + + @register() def sigwinch_untrapped_external(sh): 'untrapped SIGWINCH during external command'