Commit d64a48db authored by Michael Achenbach's avatar Michael Achenbach Committed by Commit Bot

[testrunner] Prevent erroneous overriding of signal handlers

When an overall timeout is reached, swarming sends a SIGTERM to
terminate the test runner. The test runner has a signal handler on the
main process to terminate all workers gracefully.

Additionally, every worker process installs a signal handler for
terminating ongoing tests wrapped by command.Command.

Also, command.Command is used on the main process to list tests for
cctest and gtest executables, which led to overriding the test runner's
main signal handler.

This CL disables using signal handlers in commands by default and only
explicitly enables it in safe source locations.

Bug: v8:8292
Change-Id: Ifceadaff75bdd2b77e761498bccbe00b6a3e265c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2002528Reviewed-by: 's avatarTamer Tas <tmrts@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65815}
parent 2cd24eba
...@@ -37,7 +37,8 @@ def main(args): ...@@ -37,7 +37,8 @@ def main(args):
return line return line
return None return None
cmd = command.Command(args[0], args[1:], timeout=TIMEOUT) cmd = command.Command(
args[0], args[1:], timeout=TIMEOUT, handle_sigterm=True)
previous_allocations = None previous_allocations = None
for run in range(1, MAX_TRIES + 1): for run in range(1, MAX_TRIES + 1):
......
...@@ -479,7 +479,8 @@ class RunnableConfig(GraphConfig): ...@@ -479,7 +479,8 @@ class RunnableConfig(GraphConfig):
cmd_prefix=cmd_prefix, cmd_prefix=cmd_prefix,
shell=os.path.join(shell_dir, self.binary), shell=os.path.join(shell_dir, self.binary),
args=self.GetCommandFlags(extra_flags=extra_flags), args=self.GetCommandFlags(extra_flags=extra_flags),
timeout=self.timeout or 60) timeout=self.timeout or 60,
handle_sigterm=True)
def ProcessOutput(self, output, result_tracker, count): def ProcessOutput(self, output, result_tracker, count):
"""Processes test run output and updates result tracker. """Processes test run output and updates result tracker.
......
...@@ -41,7 +41,7 @@ class AbortException(Exception): ...@@ -41,7 +41,7 @@ class AbortException(Exception):
class BaseCommand(object): class BaseCommand(object):
def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None, def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None,
verbose=False, resources_func=None): verbose=False, resources_func=None, handle_sigterm=False):
"""Initialize the command. """Initialize the command.
Args: Args:
...@@ -52,6 +52,9 @@ class BaseCommand(object): ...@@ -52,6 +52,9 @@ class BaseCommand(object):
env: Environment dict for execution. env: Environment dict for execution.
verbose: Print additional output. verbose: Print additional output.
resources_func: Callable, returning all test files needed by this command. resources_func: Callable, returning all test files needed by this command.
handle_sigterm: Flag indicating if SIGTERM will be used to terminate the
underlying process. Should not be used from the main thread, e.g. when
using a command to list tests.
""" """
assert(timeout > 0) assert(timeout > 0)
...@@ -61,6 +64,7 @@ class BaseCommand(object): ...@@ -61,6 +64,7 @@ class BaseCommand(object):
self.timeout = timeout self.timeout = timeout
self.env = env or {} self.env = env or {}
self.verbose = verbose self.verbose = verbose
self.handle_sigterm = handle_sigterm
def execute(self): def execute(self):
if self.verbose: if self.verbose:
...@@ -72,7 +76,9 @@ class BaseCommand(object): ...@@ -72,7 +76,9 @@ class BaseCommand(object):
abort_occured = [False] abort_occured = [False]
def handler(signum, frame): def handler(signum, frame):
self._abort(process, abort_occured) self._abort(process, abort_occured)
signal.signal(signal.SIGTERM, handler)
if self.handle_sigterm:
signal.signal(signal.SIGTERM, handler)
# Variable to communicate with the timer. # Variable to communicate with the timer.
timeout_occured = [False] timeout_occured = [False]
......
...@@ -271,6 +271,7 @@ class TestCase(object): ...@@ -271,6 +271,7 @@ class TestCase(object):
timeout=timeout, timeout=timeout,
verbose=self._test_config.verbose, verbose=self._test_config.verbose,
resources_func=self._get_resources, resources_func=self._get_resources,
handle_sigterm=True,
) )
def _parse_source_flags(self, source=None): def _parse_source_flags(self, source=None):
......
...@@ -191,7 +191,8 @@ class PerfTest(unittest.TestCase): ...@@ -191,7 +191,8 @@ class PerfTest(unittest.TestCase):
cmd_prefix=[], cmd_prefix=[],
shell=shell, shell=shell,
args=list(args), args=list(args),
timeout=kwargs.get('timeout', 60)) timeout=kwargs.get('timeout', 60),
handle_sigterm=True)
def _VerifyMockMultiple(self, *args, **kwargs): def _VerifyMockMultiple(self, *args, **kwargs):
self.assertEqual(len(args), len(command.Command.call_args_list)) self.assertEqual(len(args), len(command.Command.call_args_list))
...@@ -200,7 +201,8 @@ class PerfTest(unittest.TestCase): ...@@ -200,7 +201,8 @@ class PerfTest(unittest.TestCase):
'cmd_prefix': [], 'cmd_prefix': [],
'shell': os.path.join(os.path.dirname(BASE_DIR), arg[0]), 'shell': os.path.join(os.path.dirname(BASE_DIR), arg[0]),
'args': list(arg[1:]), 'args': list(arg[1:]),
'timeout': kwargs.get('timeout', 60) 'timeout': kwargs.get('timeout', 60),
'handle_sigterm': True,
} }
self.assertTupleEqual((expected, ), actual) self.assertTupleEqual((expected, ), actual)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment