Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-02-18 06:45:33 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1628527 - Introduce explicit initialization and finalization of fix-stacks. r=erahm,perftest-reviewers,sparky,gbrown

Browse Source 
Currently AWSY-with-DMD doesn't work on Windows. This is because `fix-stacks`
is initialized lazily, and by the time the initialization happens some file
descriptors for files are open, and that leads to some major Python2-on-Windows
sadness as described in the big comment in the commit.

To fix the problem, this commit adds an `init` function to `fix_stacks.py` so
that `fix-stacks` can be initialized eagerly, hopefully before any file
descriptors for files are open.

For `dmd.py`, other than fixing the AWSY problems, this has little effect,
because `fix-stacks` is always initialized.

For `utils.py`, which is used to process the output of most tests, this has a
more noticeable effect: the `fix-stacks` process is always spawned, rather than
being spawned only when needed. If no stack traces appear in the test output,
this means that `fix-stacks` is spawned unnecessarily. But it's cheap to spawn;
the expensive part only happens when stack traces start getting fixed. So I
think this change in behaviour is acceptable.

Furthermore, the commit adds a `finish` function to `fix_stacks.py`, so that
the `fix-stacks` process can be explicitly shut down. This has never been done
for processes spawned for any of the stack fixing scripts. It's never caused
problems on Linux/Mac, but it seems to be necessary on Windows to avoid
similar "this file is locked" problems with the test_dmd.js test.

The commit also renames some things to more standard Python style, e.g.
`json_mode` instead of `jsonMode`.

Finally, Android tests use `utils.py` from the repository but `fix_stacks.py`
from the Android host utils. Because the two scripts must be updated in tandem,
this commit also updates the Android host utils to a version that contains the
updated `fix_stacks.py`. Thanks to aerickson for packaging up the new Android
host utils and providing the change to the `hostutils.manifest` file.

Differential Revision: https://phabricator.services.mozilla.com/D69478
This commit is contained in:
Nicholas Nethercote 2020-04-20 22:43:25 +00:00
parent f65343cc36
commit 88cd51aa6d
5 changed files with 124 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
memory/replace/dmd/dmd.py
View File

@ -207,9 +207,9 @@ variable is used to find breakpad symbols for stack fixing.
return p.parse_args(sys.argv[1:])
# Fix stacks if necessary: first write the output to a tempfile, then replace
# the original file with it.
def fixStackTraces(inputFilename, isZipped, opener):
# Initialize stack fixing. Works much the same as fix_stacks.py:init, and the
# warnings that apply to that function apply here too.
def initStackFixing():
# This append() call is needed to make the import statements work when this
# script is installed as a symlink.
sys.path.append(os.path.dirname(__file__))
@ -217,19 +217,22 @@ def fixStackTraces(inputFilename, isZipped, opener):
bpsyms = os.environ.get('BREAKPAD_SYMBOLS_PATH', None)
sysname = platform.system()
if bpsyms and os.path.exists(bpsyms):
import fix_stacks as fixModule
def fix(line):
return fixModule.fixSymbols(line, jsonMode=True, breakpadSymsDir=bpsyms)
import fix_stacks
return fix_stacks.init(json_mode=True, breakpad_syms_dir=bpsyms)
elif sysname in ('Linux', 'Darwin', 'Windows'):
import fix_stacks as fixModule
def fix(line): return fixModule.fixSymbols(line, jsonMode=True)
import fix_stacks
return fix_stacks.init(json_mode=True)
else:
return
# No stack fixing will be performed. `fix` is the identify function,
# and `finish` does nothing.
return (lambda line: line, lambda: None)
# Fix stacks if necessary: first write the output to a tempfile, then replace
# the original file with it. `fix` must have come from `initStackTraces`.
def fixStackTraces(fix, inputFilename, isZipped, opener):
# Fix stacks, writing output to a temporary file, and then overwrite the
# original file.
tmpFile = tempfile.NamedTemporaryFile(delete=False)
@ -264,7 +267,9 @@ def getDigestFromFile(args, inputFile):
# Fix stack traces unless otherwise instructed.
if not args.no_fix_stacks:
fixStackTraces(inputFile, isZipped, opener)
(fix, finish) = initStackFixing()
fixStackTraces(fix, inputFile, isZipped, opener)
finish()
if args.clamp_contents:
clampBlockList(args, inputFile, isZipped, opener)

20
testing/awsy/awsy/awsy_test_case.py
View File

@ -126,21 +126,27 @@ class AwsyTestCase(MarionetteTestCase):
"""
Handles moving DMD reports from the temp dir to our resultsDir.
"""
from dmd import fixStackTraces
from dmd import initStackFixing, fixStackTraces
# Initialize stack fixing machinery. A single `fix-stacks` process will
# be spawned and used for every `.json.gz` file, which is good, because
# it means all the debug info will only be read once, and that's the
# slow part.
(fix, finish) = initStackFixing()
# Move DMD files from temp dir to resultsDir.
tmpdir = tempfile.gettempdir()
tmp_files = os.listdir(tmpdir)
for f in fnmatch.filter(tmp_files, "dmd-*.json.gz"):
f = os.path.join(tmpdir, f)
# We don't fix stacks on Windows, even though we could, due to the
# tale of woe in bug 1626272.
if not sys.platform.startswith('win'):
self.logger.info("Fixing stacks for %s, this may take a while" % f)
isZipped = True
fixStackTraces(f, isZipped, gzip.open)
self.logger.info("Fixing stacks for %s, this may take a while" % f)
isZipped = True
fixStackTraces(fix, f, isZipped, gzip.open)
shutil.move(f, self._resultsDir)
# Finish up the stack fixing machinery.
finish()
# Also attempt to cleanup the unified memory reports.
for f in fnmatch.filter(tmp_files, "unified-memory-report-*.json.gz"):
try:

8
testing/config/tooltool-manifests/linux64/hostutils.manifest
View File

@ -2,9 +2,9 @@
{
"algorithm": "sha512",
"visibility": "public",
"filename": "host-utils-76.0a1.en-US.linux-x86_64.tar.gz",
"filename": "host-utils-77.0a1.en-US.linux-x86_64.tar.gz",
"unpack": true,
"digest": "53026c33465466cb51f50681cb4ec19461a7fd14da6c0d3ec69fc56748cbbbce81435d54c5100a110e35ea14e5ed28b6ed71f87c9dceae9abf2cc7d49c706204",
"size": 102090412
"digest": "f52e7547e822c36e92672eb558c9b7b2856790079de549d7c3aa06b25eb78b2b7dcd429083567e016dde71d11fed5f8110573f82e94d34f3f1001b8b5eaabdda",
"size": 102067909
}
]
]

23
testing/mozbase/mozrunner/mozrunner/utils.py
View File

@ -250,21 +250,22 @@ def get_stack_fixer_function(utilityPath, symbolsPath):
# Run each line through fix_stacks.py, using breakpad symbol files.
# This method is preferred for automation, since native symbols may
# have been stripped.
stack_fixer_module = import_stack_fixer_module('fix_stacks')
def stack_fixer_function(line):
return stack_fixer_module.fixSymbols(
line, slowWarning=True, breakpadSymsDir=symbolsPath)
#
# We never call `finish` on this code path, which is a bit dodgy but
# doesn't seem to cause problems in practice.
fix_stacks = import_stack_fixer_module('fix_stacks')
(fix, finish) = fix_stacks.init(slow_warning=True, breakpad_syms_dir=symbolsPath)
return fix
elif mozinfo.isLinux or mozinfo.isMac or mozinfo.isWin:
# Run each line through fix_stacks.py. This method is preferred for
# developer machines, so we don't have to run "mach buildsymbols".
stack_fixer_module = import_stack_fixer_module('fix_stacks')
def stack_fixer_function(line):
return stack_fixer_module.fixSymbols(line, slowWarning=True)
#
# We never call `finish` on this code path, which is a bit dodgy but
# doesn't seem to cause problems in practice.
fix_stacks = import_stack_fixer_module('fix_stacks')
(fix, finish) = fix_stacks.init(slow_warning=True)
return fix
else:
return None
return stack_fixer_function

115
tools/rb/fix_stacks.py
View File

@ -20,49 +20,83 @@ line_re = re.compile("#\d+: .+\[.+ \+0x[0-9A-Fa-f]+\]")
fix_stacks = None
print_slow_warning = False
def fixSymbols(line, jsonMode=False, slowWarning=False, breakpadSymsDir=None):
# Initialize stack-fixing machinery. Spawns a `fix-stacks` process that will
# persist as long as this Python process does. Returns a pair `(fix, finish)`
# `fix` is the function to pass each fixable line; `finish` is the function to
# call once fixing is finished.
#
# WARNING: on Windows, when this function is called, the spawned `fix-stacks`
# process will inherit any open file descriptors. This can cause problems as
# seen in bug 1626272. Specifically, if file F has a file descriptor open when
# this function is called, `fix-stacks` will inherit that file descriptor, and
# subsequent attempts to access F from Python code can cause errors of the form
# "The process cannot access the file because it is being used by another
# process". Therefore, this function should be called without any file
# descriptors being open.
#
# This appears to be a Windows-only, Python2-only problem. In Python3, file
# descriptors are non-inheritable by default on all platforms, and `Popen` has
# `close_fds=True` as the default, either of which would avoid the problem.
# Unfortunately in Python2 on Windows, file descriptors are inheritable, and
# `close_fds=True` is unusable (as the `subprocess` docs explain: "on Windows,
# you cannot set `close_fds` to true and also redirect the standard handles by
# setting `stdin`, `stdout` or `stderr`", and we need to redirect those
# standard handles). All this is badly broken, but we have to work around it.
def init(json_mode=False, slow_warning=False, breakpad_syms_dir=None):
global fix_stacks
global print_slow_warning
# Look in MOZ_FETCHES_DIR (for automation), then in MOZBUILD_STATE_PATH
# (for a local build where the user has that set), then in ~/.mozbuild (for
# a local build with default settings).
base = os.environ.get(
'MOZ_FETCHES_DIR',
os.environ.get(
'MOZBUILD_STATE_PATH',
os.path.expanduser('~/.mozbuild')
)
)
fix_stacks_exe = base + '/fix-stacks/fix-stacks'
if platform.system() == 'Windows':
fix_stacks_exe = fix_stacks_exe + '.exe'
if not (os.path.isfile(fix_stacks_exe) and os.access(fix_stacks_exe, os.X_OK)):
raise Exception('cannot find `fix-stacks`; please run `./mach bootstrap`')
args = [fix_stacks_exe]
if json_mode:
args.append('-j')
if breakpad_syms_dir:
# `fileid` should be packaged next to `fix_stacks.py`.
here = os.path.dirname(__file__)
fileid_exe = os.path.join(here, 'fileid')
if platform.system() == 'Windows':
fileid_exe = fileid_exe + '.exe'
args.append('-b')
args.append(breakpad_syms_dir + "," + fileid_exe)
fix_stacks = Popen(args, stdin=PIPE, stdout=PIPE, stderr=None)
print_slow_warning = slow_warning
return (fix, finish)
def fix(line):
global print_slow_warning
result = line_re.search(line)
if result is None:
return line
if not fix_stacks:
# Look in MOZ_FETCHES_DIR (for automation), then in MOZBUILD_STATE_PATH
# (for a local build where the user has that set), then in ~/.mozbuild
# (for a local build with default settings).
base = os.environ.get(
'MOZ_FETCHES_DIR',
os.environ.get(
'MOZBUILD_STATE_PATH',
os.path.expanduser('~/.mozbuild')
)
)
fix_stacks_exe = base + '/fix-stacks/fix-stacks'
if platform.system() == 'Windows':
fix_stacks_exe = fix_stacks_exe + '.exe'
if not (os.path.isfile(fix_stacks_exe) and os.access(fix_stacks_exe, os.X_OK)):
raise Exception('cannot find `fix-stacks`; please run `./mach bootstrap`')
args = [fix_stacks_exe]
if jsonMode:
args.append('-j')
if breakpadSymsDir:
# `fileid` should be packaged next to `fix_stacks.py`.
here = os.path.dirname(__file__)
fileid_exe = os.path.join(here, 'fileid')
if platform.system() == 'Windows':
fileid_exe = fileid_exe + '.exe'
args.append('-b')
args.append(breakpadSymsDir + "," + fileid_exe)
fix_stacks = Popen(args, stdin=PIPE, stdout=PIPE, stderr=None)
if slowWarning:
print("Initializing stack-fixing for the first stack frame, this may take a while...")
# Note that this warning isn't printed until we hit our first stack frame
# that requires fixing.
if print_slow_warning:
print("Initializing stack-fixing for the first stack frame, this may take a while...")
print_slow_warning = False
# Sometimes `line` is lacking a trailing newline. If we pass such a `line`
# to `fix-stacks` it will wait until it receives a newline, causing this
@ -79,6 +113,13 @@ def fixSymbols(line, jsonMode=False, slowWarning=False, breakpadSymsDir=None):
return out
# Shut down the `fix-stacks` process.
def finish():
fix_stacks.terminate()
if __name__ == "__main__":
init()
for line in sys.stdin:
sys.stdout.write(fixSymbols(line))
sys.stdout.write(fix(line))
finish()