From 3ab4e69b1a267ce46bce9bd480ef435ec743f05b Mon Sep 17 00:00:00 2001
From: Andy C <andy@lenny>
Date: Fri, 21 Jan 2022 19:56:54 -0500
Subject: [PATCH] [interactive] Ignore SIGWINCH in the shell.

The line editor is the only thing that needs to know the width, so I
think we should let it handle the problem.

Unfortunately GNU readline is not what we want, but that's a separate
issue.

This means the core/comp_ui.py now doesn't adjust when the window is
resized.  But that dosn't make the shell unusuable.  We should revisit
it later.

Apparently fish and zsh do something different anyway -- they REDRAW on
SIGWINCH.  They don't merely update the width.

- Added a test in test/interactive.py.
- Addresses #1067
---
 core/comp_ui.py     |  5 +++++
 core/pyos.py        |  5 ++++-
 test/interactive.py | 32 ++++++++++++++++++++++++--------
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/core/comp_ui.py b/core/comp_ui.py
index 187e3ed827..8228003287 100644
--- a/core/comp_ui.py
+++ b/core/comp_ui.py
@@ -518,6 +518,11 @@ def _GetTerminalWidth(self):
 
   def OnWindowChange(self):
     # type: () -> None
+    """
+    Note: SIGWINCH handler in core/pyos.py no longer calls this.
+    The shell process should probably handle SIGWINCH only when blocked in
+    readline().
+    """
     # Only do it for the NEXT completion.  The signal handler can be run in
     # between arbitrary bytecodes, and we don't want a single completion
     # display to be shown with different widths.
diff --git a/core/pyos.py b/core/pyos.py
index 4e97376ab6..5db0f9dbc4 100644
--- a/core/pyos.py
+++ b/core/pyos.py
@@ -282,7 +282,10 @@ def InitInteractiveShell(self, display):
 
     # Register a callback to receive terminal width changes.
     # NOTE: In line_input.c, we turned off rl_catch_sigwinch.
-    signal.signal(signal.SIGWINCH, lambda x, y: display.OnWindowChange())
+    #signal.signal(signal.SIGWINCH, lambda x, y: display.OnWindowChange())
+
+    # Disabled because it causes wait builtin to abort (issue 1067)
+    signal.signal(signal.SIGWINCH, signal.SIG_IGN)
 
   def AddUserTrap(self, sig_num, handler):
     # type: (int, Any) -> None
diff --git a/test/interactive.py b/test/interactive.py
index 6fee4ec025..0f6bca42bf 100755
--- a/test/interactive.py
+++ b/test/interactive.py
@@ -42,22 +42,22 @@ def get_pid_by_name(name):
   # XXX: make sure this is restricted to subprocesses under us.
   # This could be problematic on the continuous build if many tests are running
   # in parallel.
-  output = pexpect.run('pgrep --exact --newest cat')
+  output = pexpect.run('pgrep --exact --newest %s' % name)
   return int(output.split()[-1])
 
 
+def send_signal(name, sig_num):
+  """Kill the most recent process matching `name`."""
+  os.kill(get_pid_by_name(name), sig_num)
+
+
 # XXX: osh.sendcontrol("z") does not suspend the foreground process :(
 #
 # why does osh.sendcontrol("c") generate SIGINT, while osh.sendcontrol("z")
 # appears to do nothing?
 def stop_process__hack(name):
   """Send sigstop to the most recent process matching `name`"""
-  os.kill(get_pid_by_name(name), signal.SIGSTOP)
-
-
-def kill_process(name):
-  """Kill the most recent process matching `name`."""
-  os.kill(get_pid_by_name(name), signal.SIGINT)
+  send_signal(name, signal.SIGSTOP)
 
 
 class InteractiveTest(object):
@@ -119,6 +119,22 @@ def __exit__(self, t, v, tb):
 
 
 def main(argv):
+  with InteractiveTest('wait builtin then SIGWINCH (issue 1067)') as osh:
+    osh.sendline('sleep 1 &')
+    osh.sendline('wait')
+
+    time.sleep(0.1)
+
+    # simulate window size change
+    osh.kill(signal.SIGWINCH)
+
+    osh.expect(r'.*\$')  # expect prompt
+
+    osh.sendline('echo status=$?')
+    osh.expect('status=0')
+
+  #return
+
   with InteractiveTest('Ctrl-C during external command') as osh:
     osh.sendline('sleep 5')
 
@@ -201,7 +217,7 @@ def main(argv):
     osh.expect(r".*\$")
     osh.sendline("fg")
     osh.expect(r"Continue PID \d+")
-    kill_process("cat")
+    send_signal("cat", signal.SIGINT)
     osh.expect(r".*\$")
     osh.sendline("fg")
     osh.expect("No job to put in the foreground")