From 985de3c3aae243dc5cf9a4fc2b3d73583e7b3dcd Mon Sep 17 00:00:00 2001 From: Andy C Date: Wed, 26 Jan 2022 19:27:35 -0500 Subject: [PATCH] [signal-handling] Fix exit code of 'wait' on trapped signal. This completes a fix for 'wait builtin should be interruptible'. That wasn't entirely correct. Also add tests case for SIGINT. - SIGINT when trapped correctly gives status=130 - SIGINT when not trapped doesn't give the right status. This has to do with KeyboardInterrupt. So yes we should always trap it! This is #896. Also suppress color in test/interactive then output isn't a TTY. --- osh/builtin_process.py | 11 +++++--- test/interactive.py | 59 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/osh/builtin_process.py b/osh/builtin_process.py index ed0517efb3..06838845fc 100644 --- a/osh/builtin_process.py +++ b/osh/builtin_process.py @@ -155,6 +155,7 @@ def _Run(self, cmd_val): if len(job_ids) == 0: #log('*** wait') + status = 0 i = 0 while True: # BUG: If there is a STOPPED process, this will hang forever, because @@ -162,13 +163,17 @@ def _Run(self, cmd_val): # Not sure it matters since you can now Ctrl-C it. result = self.waiter.WaitForOne(False) - if result != 0: - break # nothing to wait for, or interrupted + if result == -1: # nothing to wait for, or interrupted. status is 0 + break + elif result > 128: # signal + status = result + break + i += 1 if self.job_state.NoneAreRunning(): break - return 0 if result == -1 else result + return status # Get list of jobs. Then we need to check if they are ALL stopped. # Returns the exit code of the last one on the COMMAND LINE, not the exit diff --git a/test/interactive.py b/test/interactive.py index 0946dfacbd..c26325d1d4 100755 --- a/test/interactive.py +++ b/test/interactive.py @@ -119,10 +119,50 @@ def decorator(func): # 3. trap 'echo X' SIGINT ? +@register() +def trapped_1(sh): + 'trapped SIG during wait builtin' + + sh.sendline("trap 'echo HUP' HUP") + sh.sendline('sleep 1 &') + sh.sendline('wait') + + time.sleep(0.1) + + # simulate window size change + sh.kill(signal.SIGHUP) + + sh.expect(r'.*\$') # expect prompt + + sh.sendline('echo status=$?') + sh.expect('status=129') + + +@register() +def trapped_sigint(sh): + 'trapped SIGINT during wait builtin' + + # This is different than Ctrl-C during wait builtin, because it's trapped! + + sh.sendline("trap 'echo INT' INT") + sh.sendline('sleep 1 &') + sh.sendline('wait') + + time.sleep(0.1) + + # simulate window size change + sh.kill(signal.SIGINT) + + sh.expect(r'.*\$') # expect prompt + + sh.sendline('echo status=$?') + sh.expect('status=130') + + # TODO: Make this pass in OSH @register() -def t0(sh): - 'wait builtin then SIGWINCH (issue 1067)' +def sigwinch_1(sh): + 'SIGWINCH during wait builtin (issue 1067)' sh.sendline('sleep 1 &') sh.sendline('wait') @@ -379,7 +419,16 @@ def RunCases(cases, case_predicate, shell_pairs, results): def PrintResults(shell_pairs, results): - f = sys.stderr + f = sys.stdout + + if f.isatty(): + fail_color = ansi.BOLD + ansi.RED + ok_color = ansi.BOLD + ansi.GREEN + reset = ansi.RESET + else: + fail_color = '' + ok_color = '' + reset = '' f.write('\n') @@ -401,9 +450,9 @@ def PrintResults(shell_pairs, results): f.write('SKIP\t') elif cell == Result.FAIL: status = 1 - f.write('%sFAIL%s\t' % (ansi.BOLD + ansi.RED, ansi.RESET)) + f.write('%sFAIL%s\t' % (fail_color, reset)) elif cell == Result.OK: - f.write('%sok%s\t' % (ansi.BOLD + ansi.GREEN, ansi.RESET)) + f.write('%sok%s\t' % (ok_color, reset)) else: raise AssertionError(cell)