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

[FUJIWARA Katsunori]

At Mon, 01 May 2017 19:33:23 +0900,
FUJIWARA Katsunori wrote:
> 
> # 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

Oops, I overlooked that self._testlock() returns None if lock is
removed for dead locker. I'll send revised series.

>                      if locker is not None:
>                          raise error.LockHeld(errno.EAGAIN,
>                                               self.vfs.join(self.f), self.desc,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-- 
----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp