Skip to content

Commit

Permalink
test: add --abort-on-timeout option to test.py
Browse files Browse the repository at this point in the history
Currently, when a process times out, it is terminated by sending it the
SIGTERM signal. Sending SIGBART instead allows the operating system to
generate a core file that can be investigated later using post-mortem
debuggers such as llnode or mdb_v8.

This can be very useful when investigating flaky tests that time out,
since in that case the failure is difficult to reproduce, and being able
to look at a core file makes a big difference.

With these changes, passing the --abort-on-timeout command line option
to tools/test.py now sends SIGABRT to processes timing out on all
platforms but Windows.

PR-URL: nodejs#11086
Ref: nodejs#11026
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
Julien Gilli committed Feb 13, 2017
1 parent 6034bdc commit 82dab73
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions tools/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,11 +564,11 @@ def HasFailed(self):
return execution_failed


def KillProcessWithID(pid):
def KillProcessWithID(pid, signal_to_send=signal.SIGTERM):
if utils.IsWindows():
os.popen('taskkill /T /F /PID %d' % pid)
else:
os.kill(pid, signal.SIGTERM)
os.kill(pid, signal_to_send)


MAX_SLEEP_TIME = 0.1
Expand All @@ -587,6 +587,17 @@ def Win32SetErrorMode(mode):
pass
return prev_error_mode


def KillTimedOutProcess(context, pid):
signal_to_send = signal.SIGTERM
if context.abort_on_timeout:
# Using SIGABRT here allows the OS to generate a core dump that can be
# looked at post-mortem, which helps for investigating failures that are
# difficult to reproduce.
signal_to_send = signal.SIGABRT
KillProcessWithID(pid, signal_to_send)


def RunProcess(context, timeout, args, **rest):
if context.verbose: print "#", " ".join(args)
popen_args = args
Expand Down Expand Up @@ -626,7 +637,7 @@ def RunProcess(context, timeout, args, **rest):
while True:
if time.time() >= end_time:
# Kill the process and wait for it to exit.
KillProcessWithID(process.pid)
KillTimedOutProcess(context, process.pid)
exit_code = process.wait()
timed_out = True
break
Expand All @@ -647,7 +658,7 @@ def RunProcess(context, timeout, args, **rest):
while exit_code is None:
if (not end_time is None) and (time.time() >= end_time):
# Kill the process and wait for it to exit.
KillProcessWithID(process.pid)
KillTimedOutProcess(context, process.pid)
exit_code = process.wait()
timed_out = True
else:
Expand Down Expand Up @@ -855,7 +866,7 @@ class Context(object):

def __init__(self, workspace, buildspace, verbose, vm, expect_fail,
timeout, processor, suppress_dialogs,
store_unexpected_output, repeat):
store_unexpected_output, repeat, abort_on_timeout):
self.workspace = workspace
self.buildspace = buildspace
self.verbose = verbose
Expand All @@ -866,6 +877,7 @@ def __init__(self, workspace, buildspace, verbose, vm, expect_fail,
self.suppress_dialogs = suppress_dialogs
self.store_unexpected_output = store_unexpected_output
self.repeat = repeat
self.abort_on_timeout = abort_on_timeout

def GetVm(self, arch, mode):
if arch == 'none':
Expand Down Expand Up @@ -1400,6 +1412,9 @@ def BuildOptions():
result.add_option('--repeat',
help='Number of times to repeat given tests',
default=1, type="int")
result.add_option('--abort-on-timeout',
help='Send SIGABRT instead of SIGTERM to kill processes that time out',
default=False, dest="abort_on_timeout")
return result


Expand Down Expand Up @@ -1579,7 +1594,8 @@ def Main():
processor,
options.suppress_dialogs,
options.store_unexpected_output,
options.repeat)
options.repeat,
options.abort_on_timeout)
# First build the required targets
if not options.no_build:
reqs = [ ]
Expand Down

0 comments on commit 82dab73

Please sign in to comment.