From 2dcde5579bb418b0831f0799bde31afee40d40f1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 26 Mar 2024 12:40:37 -0500 Subject: [PATCH] Fixed issue with test.py/bench.py -f/--fail not killing runners While the -f/--fail logic was correctly terminating the test.py/bench.py runner thread, it was not terminating the actual underlying test process. This was causing test.py/bench.py to hang until the test runner completed all pending tests, which could take quite some time. This wasn't noticed earlier because test.py/bench.py still reports the test as failed, and most uses of -f/--fail involve specifying a specific test case, which usually terminates quite quickly. What's more interesting is this termination logic was copied from the handling of ctrl-C/SIGINT/KeyboardInterrupt, but this issue is not present there because SIGINT would be sent to all processes in the process tree, terminating the child process anyways. Fixed by adding an explicit proc.kill() to test.py/bench.py before tearing down the runner thread. --- scripts/bench.py | 2 ++ scripts/test.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/scripts/bench.py b/scripts/bench.py index de91cb19..8d12af22 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -1010,6 +1010,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): elif op == 'finished': # force a failure if args.get('fail'): + proc.kill() raise BenchFailure(last_id, 0, list(last_stdout)) # passed case = m.group('case') @@ -1065,6 +1066,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): proged += proged_ erased += erased_ except KeyboardInterrupt: + proc.kill() raise BenchFailure(last_id, 0, list(last_stdout)) finally: children.remove(proc) diff --git a/scripts/test.py b/scripts/test.py index 5a2388c0..7acadb29 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -1005,6 +1005,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): elif op == 'finished': # force a failure? if args.get('fail'): + proc.kill() raise TestFailure(last_id, 0, list(last_stdout)) # passed case = m.group('case') @@ -1032,6 +1033,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): if args.get('keep_going'): proc.kill() except KeyboardInterrupt: + proc.kill() raise TestFailure(last_id, 0, list(last_stdout)) finally: children.remove(proc)