[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

@@ -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

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):

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

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'