[PATCH 1 of 4 STABLE V2] lock: avoid unintentional lock acquisition at failure of readlock

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Mon May 1 10:33:23 UTC 2017


# HG changeset patch
# User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
# Date 1493634412 -32400
#      Mon May 01 19:26:52 2017 +0900
# Branch stable
# Node ID 1a8c526828d5c2e86800c1680dec5cb4660f2af7
# Parent  b59a292d0a536ee21e17d018829c36d8d4415569
lock: avoid unintentional lock acquisition at failure of readlock

Acquiring lock by vfs.makelock() and getting lock holder (aka
"locker") information by vfs.readlock() aren't atomic action.
Therefore, failure of the former doesn't ensure success of the latter.

Before this patch, lock is unintentionally acquired, if
self.parentlock is None (this is default value), and lock._readlock()
returns None for ENOENT at vfs.readlock(), because these None are
recognized as equal to each other.

In this case, lock symlink (or file) isn't created, even though lock
is treated as acquired in memory.

To avoid this issue, this patch retries lock acquisition immediately,
if lock._readlock() returns None "locker".

This issue will be covered by a test added in subsequent patch,
because simple emulation of ENOENT at vfs.readlock() easily causes
another issue.  "raising ENOENT only at the first vfs.readlock()
invocation" is too complicated for unit test, IMHO.

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -131,6 +131,9 @@ class lock(object):
             except (OSError, IOError) as why:
                 if why.errno == errno.EEXIST:
                     locker = self._readlock()
+                    if locker is None:
+                        continue
+
                     # special case where a parent process holds the lock -- this
                     # is different from the pid being different because we do
                     # want the unlock and postrelease functions to be called,
@@ -140,6 +143,9 @@ class lock(object):
                         self.held = 1
                         return
                     locker = self._testlock(locker)
+                    # keep checking "locker" for safety, even though
+                    # current _testlock doesn't return None for
+                    # non-None locker
                     if locker is not None:
                         raise error.LockHeld(errno.EAGAIN,
                                              self.vfs.join(self.f), self.desc,


More information about the Mercurial-devel mailing list