Bug 1625888 - In Talos, check_for_crashes even when exception raised after browser run; r=perftest-reviewers,AlexandruIonescu

In some cases, a browser crash may subsequently cause an exception in the harness before
check_for_crashes is called, effectively bypassing crash reporting. This patch catches
the exception to ensure that check_for_crashes is called regardless of such exceptions.
It also fixes the check for non-unicode file names and adds more diagnostic logging.

Differential Revision: https://phabricator.services.mozilla.com/D68901

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Geoff Brown 2020-04-03 13:09:05 +00:00
parent 84bf0fdb08
commit d0dc11cfdb
2 changed files with 78 additions and 67 deletions

View File

@ -492,6 +492,7 @@ if mozinfo.isWin:
FILE_ATTRIBUTE_NORMAL = 0x80 FILE_ATTRIBUTE_NORMAL = 0x80
INVALID_HANDLE_VALUE = -1 INVALID_HANDLE_VALUE = -1
log = get_logger()
file_name = os.path.join(dump_directory, file_name = os.path.join(dump_directory,
str(uuid.uuid4()) + ".dmp") str(uuid.uuid4()) + ".dmp")
@ -501,7 +502,6 @@ if mozinfo.isWin:
# python process was compiled for a different architecture than # python process was compiled for a different architecture than
# firefox, so we invoke the minidumpwriter utility program. # firefox, so we invoke the minidumpwriter utility program.
log = get_logger()
minidumpwriter = os.path.normpath(os.path.join(utility_path, minidumpwriter = os.path.normpath(os.path.join(utility_path,
"minidumpwriter.exe")) "minidumpwriter.exe"))
log.info(u"Using {} to write a dump to {} for [{}]".format( log.info(u"Using {} to write a dump to {} for [{}]".format(
@ -519,12 +519,16 @@ if mozinfo.isWin:
log.error("minidumpwriter exited with status: %d" % status) log.error("minidumpwriter exited with status: %d" % status)
return return
log.info(u"Writing a dump to {} for [{}]".format(file_name, pid))
proc_handle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, proc_handle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,
0, pid) 0, pid)
if not proc_handle: if not proc_handle:
err = kernel32.GetLastError()
log.warning("unable to get handle for pid %d: %d" % (pid, err))
return return
if not isinstance(file_name, string_types): if not isinstance(file_name, text_type):
# Convert to unicode explicitly so our path will be valid as input # Convert to unicode explicitly so our path will be valid as input
# to CreateFileW # to CreateFileW
file_name = text_type(file_name, sys.getfilesystemencoding()) file_name = text_type(file_name, sys.getfilesystemencoding())
@ -537,18 +541,23 @@ if mozinfo.isWin:
FILE_ATTRIBUTE_NORMAL, FILE_ATTRIBUTE_NORMAL,
None) None)
if file_handle != INVALID_HANDLE_VALUE: if file_handle != INVALID_HANDLE_VALUE:
ctypes.windll.dbghelp.MiniDumpWriteDump(proc_handle, if not ctypes.windll.dbghelp.MiniDumpWriteDump(proc_handle,
pid, pid,
file_handle, file_handle,
# Dump type - MiniDumpNormal # Dump type - MiniDumpNormal
0, 0,
# Exception parameter # Exception parameter
None, None,
# User stream parameter # User stream parameter
None, None,
# Callback parameter # Callback parameter
None) None):
err = kernel32.GetLastError()
log.warning("unable to dump minidump file for pid %d: %d" % (pid, err))
CloseHandle(file_handle) CloseHandle(file_handle)
else:
err = kernel32.GetLastError()
log.warning("unable to create minidump file for pid %d: %d" % (pid, err))
CloseHandle(proc_handle) CloseHandle(proc_handle)
def kill_pid(pid): def kill_pid(pid):

View File

@ -223,67 +223,69 @@ class TTest(object):
if counter_management: if counter_management:
counter_management.stop() counter_management.stop()
if test_config['mainthread']: try:
rawlog = os.path.join(here, 'mainthread_io.log') if test_config['mainthread']:
if os.path.exists(rawlog): rawlog = os.path.join(here, 'mainthread_io.log')
processedlog = \ if os.path.exists(rawlog):
os.path.join(here, 'mainthread_io.json') processedlog = \
xre_path = \ os.path.join(here, 'mainthread_io.json')
os.path.dirname(browser_config['browser_path']) xre_path = \
mtio_py = os.path.join(here, 'mainthreadio.py') os.path.dirname(browser_config['browser_path'])
command = ['python', mtio_py, rawlog, mtio_py = os.path.join(here, 'mainthreadio.py')
processedlog, xre_path] command = ['python', mtio_py, rawlog,
mtio = subprocess.Popen(command, processedlog, xre_path]
env=os.environ.copy(), mtio = subprocess.Popen(command,
stdout=subprocess.PIPE) env=os.environ.copy(),
output, stderr = mtio.communicate() stdout=subprocess.PIPE)
for line in output.split('\n'): output, stderr = mtio.communicate()
if line.strip() == '': for line in output.split('\n'):
continue if line.strip() == '':
continue
print(line) print(line)
mainthread_error_count += 1 mainthread_error_count += 1
mozfile.remove(rawlog) mozfile.remove(rawlog)
if test_config['cleanup']: if test_config['cleanup']:
# HACK: add the pid to support xperf where we require # HACK: add the pid to support xperf where we require
# the pid in post processing # the pid in post processing
talosconfig.generateTalosConfig(command_args, talosconfig.generateTalosConfig(command_args,
browser_config, browser_config,
test_config, test_config,
pid=pcontext.pid) pid=pcontext.pid)
subprocess.call( subprocess.call(
[sys.executable] + test_config['cleanup'].split() [sys.executable] + test_config['cleanup'].split()
) )
# For startup tests, we launch the browser multiple times # For startup tests, we launch the browser multiple times
# with the same profile # with the same profile
for fname in ('sessionstore.js', '.parentlock', for fname in ('sessionstore.js', '.parentlock',
'sessionstore.bak'): 'sessionstore.bak'):
mozfile.remove(os.path.join(setup.profile_dir, fname)) mozfile.remove(os.path.join(setup.profile_dir, fname))
# check for xperf errors # check for xperf errors
if os.path.exists(browser_config['error_filename']) or \ if os.path.exists(browser_config['error_filename']) or \
mainthread_error_count > 0: mainthread_error_count > 0:
raise TalosRegression( raise TalosRegression(
'Talos has found a regression, if you have questions' 'Talos has found a regression, if you have questions'
' ask for help in irc on #perf' ' ask for help in irc on #perf'
) )
# add the results from the browser output # add the results from the browser output
if not run_in_debug_mode(browser_config): if not run_in_debug_mode(browser_config):
test_results.add( test_results.add(
'\n'.join(pcontext.output), '\n'.join(pcontext.output),
counter_results=(counter_management.results() counter_results=(counter_management.results()
if counter_management if counter_management
else None) else None)
) )
if setup.gecko_profile: if setup.gecko_profile:
setup.gecko_profile.symbolicate(i) setup.gecko_profile.symbolicate(i)
self.check_for_crashes(browser_config, minidump_dir, finally:
test_config['name']) self.check_for_crashes(browser_config, minidump_dir,
test_config['name'])
# include global (cross-cycle) counters # include global (cross-cycle) counters
test_results.all_counter_results.extend( test_results.all_counter_results.extend(