Bug 1576748 - make symbolstore.py complain loudly if expected output isn't found; r=nalexander
☠☠ backed out by 10f791c3a57f ☠ ☠
authorNathan Froyd <froydnj@mozilla.com>
Fri, 30 Aug 2019 15:37:13 +0000
changeset 551545 97ec7ec089c4e91acd34d7324711cc9c556b1117
parent 551544 3a41fd305c3ce85b9f534f15cd90dfd4ae82c0d1
child 551546 19b8d6a399f60de32ca253fa497c23cbfa93d265
push id11865
push userbtara@mozilla.com
push dateMon, 02 Sep 2019 08:54:37 +0000
treeherdermozilla-beta@37f59c4671b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1576748
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1576748 - make symbolstore.py complain loudly if expected output isn't found; r=nalexander This change surfaces errors faster, and ensures that we don't silently end up with empty crashreporter symbols if `dump_syms` can't run for some reason. Differential Revision: https://phabricator.services.mozilla.com/D43520
toolkit/crashreporter/tools/symbolstore.py
--- a/toolkit/crashreporter/tools/symbolstore.py
+++ b/toolkit/crashreporter/tools/symbolstore.py
@@ -519,19 +519,31 @@ class Dumper:
         t_start = time.time()
         print("Processing file: %s" % file, file=sys.stderr)
 
         sourceFileStream = ''
         code_id, code_file = None, None
         try:
             cmd = self.dump_syms_cmdline(file, arch, dsymbundle=dsymbundle)
             print(' '.join(cmd), file=sys.stderr)
+            # We're interested in `stderr` in the case that something goes
+            # wrong with dump_syms, but we don't want to use
+            # `stderr=subprocess.PIPE` here, as that can land us in a
+            # deadlock when we try to read only from `stdout`, below.  The
+            # Python documentation recommends using `communicate()` in such
+            # cases, but `stderr` can be rather large, and we don't want to
+            # waste time accumulating all of it in the non-error case.  So we
+            # completely ignore `stderr` here and capture it separately,
+            # below.
             proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                                     stderr=open(os.devnull, 'wb'))
-            module_line = proc.stdout.next()
+            try:
+                module_line = proc.stdout.next()
+            except StopIteration:
+                module_line = ''
             if module_line.startswith("MODULE"):
                 # MODULE os cpu guid debug_file
                 (guid, debug_file) = (module_line.split())[3:5]
                 # strip off .pdb extensions, and append .sym
                 sym_file = re.sub("\.pdb$", "", debug_file) + ".sym"
                 # we do want forward slashes here
                 rel_path = os.path.join(debug_file,
                                         guid,
@@ -601,18 +613,32 @@ class Dumper:
                 print(rel_path)
                 if self.srcsrv and vcs_root:
                     # add source server indexing to the pdb file
                     self.SourceServerIndexing(debug_file, guid, sourceFileStream, vcs_root)
                 # only copy debug the first time if we have multiple architectures
                 if self.copy_debug and arch_num == 0:
                     self.CopyDebug(file, debug_file, guid,
                                    code_file, code_id)
-        except StopIteration:
-            pass
+            else:
+                # For some reason, we didn't see the MODULE line as the first
+                # line of output.  It's very possible that the interesting error
+                # message(s) are on stderr, so let's re-execute the process and
+                # capture the entirety of stderr.
+                proc = subprocess.Popen(cmd, stdout=open(os.devnull, 'wb'),
+                                        stderr=subprocess.PIPE)
+                (_, dumperr) = proc.communicate()
+                retcode = proc.returncode
+                message = [
+                    "dump_syms failed to produce the expected output",
+                    "return code: %d" % retcode,
+                    "first line of output: %s" % module_line,
+                    "stderr: %s" % dumperr
+                ]
+                raise RuntimeError('\n----------\n'.join(message))
         except Exception as e:
             print("Unexpected error: %s" % str(e), file=sys.stderr)
             raise
 
         if dsymbundle:
             shutil.rmtree(dsymbundle)
 
         if count_ctors: