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>
diff --git a/binary_search_tool/bisect_driver.py b/binary_search_tool/bisect_driver.py
index 6c4d406..1f7babd 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 @@
from __future__ import print_function
+import contextlib
+import fcntl
import os
import shutil
import subprocess
@@ -34,13 +35,49 @@
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 @@
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 @@
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 @@
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 @@
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
