Commit b16aeb42 authored by erikchen's avatar erikchen Committed by Commit Bot

Context TearDown should not invoke ADB if process receives SIGTERM.

The TearDown method on several contexts will invokes ADB. In some instances
[e.g. on receiving a SIGTERM due to timeout] there's a high probability that ADB
is non-responsive. In these cases, sending an ADB command will potentially take
a long time to time out. Before this happens, the process will be hard-killed
for not responding to SIGTERM fast enough.

This CL makes the test_runner.py SIGTERM handler set
allow_adb_on_teardown=False. This prevents these contexts from invoking ADB,
increasing the likelihood that SIGTERM will handled within the 30 second limit
and emit JSON test results.

Bug: 895027
Change-Id: Ic30cff73a82a5415c5f531eae5c8f62fb4c953aa
Reviewed-on: https://chromium-review.googlesource.com/c/1311259Reviewed-by: default avatarJohn Budorick <jbudorick@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604584}
parent e53c13fd
...@@ -25,6 +25,9 @@ class Environment(object): ...@@ -25,6 +25,9 @@ class Environment(object):
""" """
self._output_manager = output_manager self._output_manager = output_manager
# Some subclasses have different teardown behavior on receiving SIGTERM.
self._received_sigterm = False
def SetUp(self): def SetUp(self):
raise NotImplementedError raise NotImplementedError
...@@ -41,3 +44,6 @@ class Environment(object): ...@@ -41,3 +44,6 @@ class Environment(object):
@property @property
def output_manager(self): def output_manager(self):
return self._output_manager return self._output_manager
def ReceivedSigterm(self):
self._received_sigterm = True
...@@ -18,6 +18,9 @@ class TestRun(object): ...@@ -18,6 +18,9 @@ class TestRun(object):
self._env = env self._env = env
self._test_instance = test_instance self._test_instance = test_instance
# Some subclasses have different teardown behavior on receiving SIGTERM.
self._received_sigterm = False
def TestPackage(self): def TestPackage(self):
raise NotImplementedError raise NotImplementedError
...@@ -42,3 +45,6 @@ class TestRun(object): ...@@ -42,3 +45,6 @@ class TestRun(object):
def __exit__(self, exc_type, exc_val, exc_tb): def __exit__(self, exc_type, exc_val, exc_tb):
self.TearDown() self.TearDown()
def ReceivedSigterm(self):
self._received_sigterm = True
...@@ -212,6 +212,14 @@ class LocalDeviceEnvironment(environment.Environment): ...@@ -212,6 +212,14 @@ class LocalDeviceEnvironment(environment.Environment):
elif self.trace_output: elif self.trace_output:
self.DisableTracing() self.DisableTracing()
# By default, teardown will invoke ADB. When receiving SIGTERM due to a
# timeout, there's a high probability that ADB is non-responsive. In these
# cases, sending an ADB command will potentially take a long time to time
# out. Before this happens, the process will be hard-killed for not
# responding to SIGTERM fast enough.
if self._received_sigterm:
return
if not self._devices: if not self._devices:
return return
......
...@@ -603,6 +603,14 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): ...@@ -603,6 +603,14 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun):
#override #override
def TearDown(self): def TearDown(self):
# By default, teardown will invoke ADB. When receiving SIGTERM due to a
# timeout, there's a high probability that ADB is non-responsive. In these
# cases, sending an ADB command will potentially take a long time to time
# out. Before this happens, the process will be hard-killed for not
# responding to SIGTERM fast enough.
if self._received_sigterm:
return
@local_device_environment.handle_shard_failures @local_device_environment.handle_shard_failures
@trace_event.traced @trace_event.traced
def individual_device_tear_down(dev): def individual_device_tear_down(dev):
......
...@@ -305,6 +305,14 @@ class LocalDeviceInstrumentationTestRun( ...@@ -305,6 +305,14 @@ class LocalDeviceInstrumentationTestRun(
#override #override
def TearDown(self): def TearDown(self):
# By default, teardown will invoke ADB. When receiving SIGTERM due to a
# timeout, there's a high probability that ADB is non-responsive. In these
# cases, sending an ADB command will potentially take a long time to time
# out. Before this happens, the process will be hard-killed for not
# responding to SIGTERM fast enough.
if self._received_sigterm:
return
@local_device_environment.handle_shard_failures_with( @local_device_environment.handle_shard_failures_with(
self._env.BlacklistDevice) self._env.BlacklistDevice)
@trace_event.traced @trace_event.traced
......
...@@ -742,6 +742,7 @@ def RunTestsInPlatformMode(args): ...@@ -742,6 +742,7 @@ def RunTestsInPlatformMode(args):
### Set up sigterm handler. ### Set up sigterm handler.
contexts_to_notify_on_sigterm = []
def unexpected_sigterm(_signum, _frame): def unexpected_sigterm(_signum, _frame):
msg = [ msg = [
'Received SIGTERM. Shutting down.', 'Received SIGTERM. Shutting down.',
...@@ -755,6 +756,9 @@ def RunTestsInPlatformMode(args): ...@@ -755,6 +756,9 @@ def RunTestsInPlatformMode(args):
live_thread.name, live_thread.ident), live_thread.name, live_thread.ident),
thread_stack]) thread_stack])
for context in contexts_to_notify_on_sigterm:
context.ReceivedSigterm()
infra_error('\n'.join(msg)) infra_error('\n'.join(msg))
signal.signal(signal.SIGTERM, unexpected_sigterm) signal.signal(signal.SIGTERM, unexpected_sigterm)
...@@ -834,6 +838,9 @@ def RunTestsInPlatformMode(args): ...@@ -834,6 +838,9 @@ def RunTestsInPlatformMode(args):
test_run = test_run_factory.CreateTestRun( test_run = test_run_factory.CreateTestRun(
args, env, test_instance, infra_error) args, env, test_instance, infra_error)
contexts_to_notify_on_sigterm.append(env)
contexts_to_notify_on_sigterm.append(test_run)
### Run. ### Run.
with out_manager, json_finalizer(): with out_manager, json_finalizer():
with json_writer(), logcats_uploader, env, test_instance, test_run: with json_writer(), logcats_uploader, env, test_instance, test_run:
......
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