android: fix deobfuscator closing + add infinite loop test case.

Bug: 876539
Change-Id: Ib28bf55e7213789186b8180904f7317cb5c0d8ce
Reviewed-on: https://chromium-review.googlesource.com/1186542
Commit-Queue: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585843}

build/android/pylib/symbols/deobfuscator.py

@@ -25,6 +25,8 @@ class Deobfuscator(object):
     cmd = [script_path, mapping_path]
     # Allow only one thread to call TransformLines() at a time.
     self._lock = threading.Lock()
+    # Ensure that only one thread attempts to kill self._proc in Close().
+    self._close_lock = threading.Lock()
     self._closed_called = False
     # Assign to None so that attribute exists if Popen() throws.
     self._proc = None
@@ -114,8 +116,11 @@ class Deobfuscator(object):
         return lines
 
   def Close(self):
-    self._closed_called = True
+    with self._close_lock:
-    if not self.IsClosed():
+      needs_closing = not self.IsClosed()
+      self._closed_called = True
+
+    if needs_closing:
       self._proc.stdin.close()
       self._proc.kill()
       self._proc.wait()

build/android/stacktrace/java_deobfuscate_test.py

@@ -6,10 +6,14 @@
 """Tests for java_deobfuscate."""
 
 import argparse
+import os
 import subprocess
 import sys
 import tempfile
+import unittest
 
+# Set by command-line argument.
+_JAVA_DEOBFUSCATE_PATH = None
 
 LINE_PREFIXES = [
     '',
@@ -59,44 +63,75 @@ this.was.Deobfuscated: Error message
 """.splitlines(True)
 
 
-def _RunTest(bin_path, map_file, prefix):
+class JavaDeobfuscateTest(unittest.TestCase):
-  cmd = [bin_path, map_file]
-  payload = TEST_DATA
-  expected_output = EXPECTED_OUTPUT
 
-  payload = [prefix + x for x in payload]
+  def __init__(self, *args, **kwargs):
-  expected_output = [prefix + x for x in expected_output]
+    super(JavaDeobfuscateTest, self).__init__(*args, **kwargs)
+    self._map_file = None
 
-  proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+  def setUp(self):
-  actual_output = proc.communicate(''.join(payload))[0].splitlines(True)
+    self._map_file = tempfile.NamedTemporaryFile()
-  any_unexpected_failures = False
+    self._map_file.write(TEST_MAP)
-  for actual, expected in zip(actual_output, expected_output):
+    self._map_file.flush()
-    if actual == expected:
-      sys.stdout.write('Good: ' + actual)
-    elif actual.replace('bar', 'someMethod') == expected:
-      # TODO(agrieve): Fix ReTrace's ability to deobfuscated methods.
-      sys.stdout.write('Fine: ' + actual)
-    else:
-      sys.stdout.write('BAD:  ' + actual)
-      any_unexpected_failures = True
-  return not any_unexpected_failures
 
+  def tearDown(self):
+    if self._map_file:
+      self._map_file.close()
 
-def main():
+  def _testImpl(self, input_lines=None, expected_output_lines=None,
-  parser = argparse.ArgumentParser()
+                prefix=''):
-  parser.add_argument('path_to_java_deobfuscate')
+    self.assertTrue(bool(input_lines) == bool(expected_output_lines))
-  args = parser.parse_args()
 
-  with tempfile.NamedTemporaryFile() as map_file:
+    if not input_lines:
-    map_file.write(TEST_MAP)
+      input_lines = [prefix + x for x in TEST_DATA]
-    map_file.flush()
+    if not expected_output_lines:
-    passed = True
+      expected_output_lines = [prefix + x for x in EXPECTED_OUTPUT]
-    for prefix in LINE_PREFIXES:
+
-      if not _RunTest(args.path_to_java_deobfuscate, map_file.name, prefix):
+    cmd = [_JAVA_DEOBFUSCATE_PATH, self._map_file.name]
-        passed = False
+    proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
-  print 'Result:', 'PASS' if passed else 'FAIL'
+    proc_output, _ = proc.communicate(''.join(input_lines))
-  sys.exit(int(not passed))
+    actual_output_lines = proc_output.splitlines(True)
+    for actual, expected in zip(actual_output_lines, expected_output_lines):
+      self.assertTrue(
+          actual == expected or actual.replace('bar', 'someMethod') == expected,
+          msg=''.join([
+              'Deobfuscation failed.\n',
+              '  actual:   %s' % actual,
+              '  expected: %s' % expected]))
+
+  def testNoPrefix(self):
+    self._testImpl(prefix='')
+
+  def testThreadtimePrefix(self):
+    self._testImpl(prefix='09-08 14:38:35.535 18029 18084 E qcom_sensors_hal: ')
+
+  def testStandardPrefix(self):
+    self._testImpl(prefix='W/GCM     (15158): ')
+
+  def testStandardPrefixWithPadding(self):
+    self._testImpl(prefix='W/GCM     (  158): ')
+
+  @unittest.skip('causes java_deobfuscate to hang, see crbug.com/876539')
+  def testIndefiniteHang(self):
+    # Test for crbug.com/876539.
+    self._testImpl(
+        input_lines=[
+            'VFY: unable to resolve virtual method 2: LFOO;'
+                + '.onDescendantInvalidated '
+                + '(Landroid/view/View;Landroid/view/View;)V',
+        ],
+        expected_output_lines=[
+            'VFY: unable to resolve virtual method 2: Lthis.was.Deobfuscated;'
+                + '.onDescendantInvalidated '
+                + '(Landroid/view/View;Landroid/view/View;)V',
+        ])
 
 
 if __name__ == '__main__':
-  main()
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--java-deobfuscate-path', type=os.path.realpath,
+                      required=True)
+  known_args, unittest_args = parser.parse_known_args()
+  _JAVA_DEOBFUSCATE_PATH = known_args.java_deobfuscate_path
+  unittest_args = [sys.argv[0]] + unittest_args
+  unittest.main(argv=unittest_args)