Commit fc7b40d9 authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

[Supersize] Fix fragile job param binding in _BulkObjectFileAnalyzerSlave.Run().

_BulkObjectFileAnalyzerSlave.Run() is a message loop to process
AnalyzePaths() and AnalyzeStringLiterals() executions across threads.
However, jobs are created using lambdas that capture local variables
by closure. This is fragile, since if the local variables are quickly
updated, then the race condition would mutate queued jobs.

This bug is manifested by calling nm.py main() directly on multiple .o
files, using '--multiprocess' switch. This triggers the race condition,
causing only last .o file to be processed

In regular Supersize usage, there is likely sufficient delay so that
the bug is unlikely to happen -- but if it does, then we'd have
something that would be hard to debug!

The solution is to handle messages in a separate function, to isolate
contexts for local variables and closures. We chose this over
functools.partial() for simplicity (no need to chase down individual
cases).

Change-Id: Ie985807a0f19cbb4d11e1236f805da2e0a25b26b
Reviewed-on: https://chromium-review.googlesource.com/1135699
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: default avataragrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574916}
parent 80dbf61e
...@@ -657,36 +657,41 @@ class _BulkObjectFileAnalyzerSlave(object): ...@@ -657,36 +657,41 @@ class _BulkObjectFileAnalyzerSlave(object):
self._job_queue.join() self._job_queue.join()
self._allow_analyze_paths = False self._allow_analyze_paths = False
# Handle messages in a function outside the event loop, so local variables are
# independent across messages, and can be bound to jobs by lambdas using
# closures instead of functools.partial().
def _HandleMessage(self, message):
if message[0] == _MSG_ANALYZE_PATHS:
assert self._allow_analyze_paths, (
'Cannot call AnalyzePaths() after AnalyzeStringLiterals()s.')
paths = message[1].split('\x01')
self._job_queue.put(lambda: self._worker_analyzer.AnalyzePaths(paths))
elif message[0] == _MSG_SORT_PATHS:
assert self._allow_analyze_paths, (
'Cannot call SortPaths() after AnalyzeStringLiterals()s.')
self._job_queue.put(self._worker_analyzer.SortPaths)
elif message[0] == _MSG_ANALYZE_STRINGS:
self._WaitForAnalyzePathJobs()
elf_path, string_positions = message[1:]
self._job_queue.put(
lambda: self._worker_analyzer.AnalyzeStringLiterals(
elf_path, string_positions))
elif message[0] == _MSG_GET_SYMBOL_NAMES:
self._WaitForAnalyzePathJobs()
self._pipe.send(None)
paths_by_name = self._worker_analyzer.GetSymbolNames()
self._pipe.send(concurrent.EncodeDictOfLists(paths_by_name))
elif message[0] == _MSG_GET_STRINGS:
self._job_queue.join()
# Send a None packet so that other side can measure IPC transfer time.
self._pipe.send(None)
self._pipe.send(self._worker_analyzer.GetEncodedStringPositions())
def Run(self): def Run(self):
try: try:
self._worker_thread.start() self._worker_thread.start()
while True: while True:
message = self._pipe.recv() self._HandleMessage(self._pipe.recv())
if message[0] == _MSG_ANALYZE_PATHS:
assert self._allow_analyze_paths, (
'Cannot call AnalyzePaths() after AnalyzeStringLiterals()s.')
paths = message[1].split('\x01')
self._job_queue.put(lambda: self._worker_analyzer.AnalyzePaths(paths))
elif message[0] == _MSG_SORT_PATHS:
assert self._allow_analyze_paths, (
'Cannot call SortPaths() after AnalyzeStringLiterals()s.')
self._job_queue.put(self._worker_analyzer.SortPaths)
elif message[0] == _MSG_ANALYZE_STRINGS:
self._WaitForAnalyzePathJobs()
elf_path, string_positions = message[1:]
self._job_queue.put(
lambda: self._worker_analyzer.AnalyzeStringLiterals(
elf_path, string_positions))
elif message[0] == _MSG_GET_SYMBOL_NAMES:
self._WaitForAnalyzePathJobs()
self._pipe.send(None)
paths_by_name = self._worker_analyzer.GetSymbolNames()
self._pipe.send(concurrent.EncodeDictOfLists(paths_by_name))
elif message[0] == _MSG_GET_STRINGS:
self._job_queue.join()
# Send a None packet so that other side can measure IPC transfer time.
self._pipe.send(None)
self._pipe.send(self._worker_analyzer.GetEncodedStringPositions())
except EOFError: except EOFError:
pass pass
except EnvironmentError, e: except EnvironmentError, e:
......
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