浏览代码

lockfile: release the lock before closing the file handler

After crrev.com/c/6486489 landed, gclient sync failed randomly due to
lock failures. After investigation, it was concluded that the locks
are not immediately released after the lock file handler close so that
it fails to acquire the lock if gclient_scm.py attempts to lock it
immediately after mirror.populate().

In Linux, there is no guarantee that the file close will release
all the locks before processing the file description closure, unlike
the CloseHandle() do in Microsoft Windows.

This CL is to update the logic so that it releases the lock
before os.close().

Bug: 407795715
Change-Id: I0f58627d368922f27c0590dcea2e7fde4242ae17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6497038
Auto-Submit: Scott Lee <ddoman@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Scott Lee 3 月之前
父节点
当前提交
df1785bdc2
共有 1 个文件被更改,包括 19 次插入5 次删除
  1. 19 5
      lockfile.py

+ 19 - 5
lockfile.py

@@ -32,8 +32,13 @@ if sys.platform.startswith('win'):
                 None  # hTemplateFile
             ))
 
-    def _close_file(handle):
-        # CloseHandle releases lock too.
+    def _close_file(handle, unlock):
+        if unlock:
+            # Locks are released *before* the CloseHandle function is finished
+            # processing:
+            # - https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-unlockfileex#remarks
+            pass
+
         win32imports.CloseHandle(handle)
 
     def _lock_file(handle):
@@ -60,7 +65,16 @@ else:
         open_flags = (os.O_CREAT | os.O_WRONLY)
         return os.open(lockfile, open_flags, 0o644)
 
-    def _close_file(fd):
+    def _close_file(fd, unlock):
+        # "man 2 fcntl" states that closing any file descriptor referring to
+        # the lock file will release all the process locks on the file, but
+        # there is no guarantee that the locks will be released atomically
+        # before the closure.
+        #
+        # It's necessary to release the lock before the file close to avoid
+        # possible race conditions.
+        if unlock:
+            fcntl.flock(fd, fcntl.LOCK_UN)
         os.close(fd)
 
     def _lock_file(fd):
@@ -72,9 +86,9 @@ def _try_lock(lockfile):
     try:
         _lock_file(f)
     except Exception:
-        _close_file(f)
+        _close_file(f, unlock=False)
         raise
-    return lambda: _close_file(f)
+    return lambda: _close_file(f, unlock=True)
 
 
 def _lock(path, timeout=0):