binary search tool: Add file locking to compiler wrapper

Because make can run with arbitrary number of jobs, there's potential
race conditions with the compiler wrapper's file operations. Replace all
open calls with function that locks files using fnctl module.

Additionally fix small formatting errors

CQ-DEPEND=CL:*270783

Change-Id: I4b94bfe3c0a5d7203f78e87130b7c4d71bc8e1c0
Reviewed-on: https://chrome-internal-review.googlesource.com/271747
Commit-Ready: Cassidy Burden <cburden@google.com>
Tested-by: Cassidy Burden <cburden@google.com>
Reviewed-by: Luis Lozano <llozano@chromium.org>

1 files changed, 46 insertions, 9 deletions

diff --git a/binary_search_tool/bisect_driver.py b/binary_search_tool/bisect_driver.py
index 6c4d406c..1f7babd9 100644
--- a/binary_search_tool/bisect_driver.py
+++ b/binary_search_tool/bisect_driver.py
@@ -2,7 +2,6 @@
 #
 # This script is used to help the compiler wrapper in the Android build system
 # bisect for bad object files.
-
 """Utilities for bisection of Android object files.
 
 This module contains a set of utilities to allow bisection between
@@ -15,6 +14,8 @@ https://docs.google.com/document/d/1yDgaUIa2O5w6dc3sSTe1ry-1ehKajTGJGQCbyn0fcEM
 
 from __future__ import print_function
 
+import contextlib
+import fcntl
 import os
 import shutil
 import subprocess
@@ -34,13 +35,49 @@ class Error(Exception):
   pass
 
 
+@contextlib.contextmanager
+def lock_file(path, mode):
+  """Lock file and block if other process has lock on file.
+
+  Acquire exclusive lock for file. Only blocks other processes if they attempt
+  to also acquire lock through this method. If only reading (modes 'r' and 'rb')
+  then the lock is shared (i.e. many reads can happen concurrently, but only one
+  process may write at a time).
+
+  This function is a contextmanager, meaning it's meant to be used with the
+  "with" statement in Python. This is so cleanup and setup happens automatically
+  and cleanly. Execution of the outer "with" statement happens at the "yield"
+  statement. Execution resumes after the yield when the outer "with" statement
+  ends.
+
+  Args:
+    path: path to file being locked
+    mode: mode to open file with ('w', 'r', etc.)
+  """
+  with open(path, mode) as f:
+    # Share the lock if just reading, make lock exclusive if writing
+    if f.mode == 'r' or f.mode == 'rb':
+      lock_type = fcntl.LOCK_SH
+    else:
+      lock_type = fcntl.LOCK_EX
+
+    try:
+      fcntl.lockf(f, lock_type)
+      yield f
+      f.flush()
+    except:
+      raise
+    finally:
+      fcntl.lockf(f, fcntl.LOCK_UN)
+
+
 def log_to_file(path, execargs, link_from=None, link_to=None):
   """Common logging function.
 
   Log current working directory, current execargs, and a from-to relationship
   between files.
   """
-  with open(path, 'a') as log:
+  with lock_file(path, 'a') as log:
     log.write('cd: %s; %s\n' % (os.getcwd(), ' '.join(execargs)))
     if link_from and link_to:
       log.write('%s -> %s\n' % (link_from, link_to))
@@ -87,14 +124,14 @@ def get_obj_path(execargs):
   try:
     i = execargs.index('-o')
   except ValueError:
-    return "", ""
+    return '', ''
 
-  obj_path = execargs[i+1]
+  obj_path = execargs[i + 1]
   if not obj_path.endswith(('.o',)):
     # TODO: what suffixes do we need to contemplate
     # TODO: add this as a warning
     # TODO: need to handle -r compilations
-    return "", ""
+    return '', ''
 
   return obj_path, os.path.join(os.getcwd(), obj_path)
 
@@ -110,9 +147,9 @@ def get_dep_path(execargs):
   try:
     i = execargs.index('-MF')
   except ValueError:
-    return "", ""
+    return '', ''
 
-  dep_path = execargs[i+1]
+  dep_path = execargs[i + 1]
   return dep_path, os.path.join(os.getcwd(), dep_path)
 
 
@@ -121,7 +158,7 @@ def in_object_list(obj_name, list_filename):
   if not obj_name:
     return False
 
-  with open(list_filename, 'r') as list_file:
+  with lock_file(list_filename, 'r') as list_file:
     for line in list_file:
       if line.strip() == obj_name:
         return True
@@ -196,7 +233,7 @@ def bisect_populate(execargs, bisect_dir, population_name):
     print('Could not populate bisect cache', file=sys.stderr)
     raise
 
-  with open(os.path.join(population_dir, '_LIST'), 'a') as object_list:
+  with lock_file(os.path.join(population_dir, '_LIST'), 'a') as object_list:
     object_list.write('%s\n' % obj_path)
 
   # Cache the side effects generated by good compiler