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

[FUJIWARA Katsunori]

# HG changeset patch
# User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
# Date 1493454422 -32400
#      Sat Apr 29 17:27:02 2017 +0900
# Branch stable
# Node ID d1bf658c4ea6d1f87328884bc00f2b91d884a6d9
# Parent  7cb8ebcca7d3a542031c41e09dc3ca0e2db36dcc
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 ENOEXIST
causes failure of vfs.readlock() while 5 times retrying, because
lock._trylock() returns to caller silently after retrying, and
lock.lock() assumes that lock._trylock() returns successfully only if
lock is acquired.

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 makes lock._trylock() raise
LockHeld(EAGAIN) at the end of it, if lock isn't acquired while
retrying.

This patch uses 0 as PID of "locker" process in the issue
case. because there is no appropriate PID for "busy for frequent
lock/unlock by many processes", even though this might confuse users
by error message "timed out waiting for lock held by 0:HOSTNAME" or
so.

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -148,6 +148,12 @@ class lock(object):
                     raise error.LockUnavailable(why.errno, why.strerror,
                                                 why.filename, self.desc)
 
+        if not self.held:
+            # use 0 to mean "busy for frequent lock/unlock by many processes"
+            locker = "%s:%d" % (lock._host, 0)
+            raise error.LockHeld(errno.EAGAIN,
+                                 self.vfs.join(self.f), self.desc, locker)
+
     def _readlock(self):
         """read lock and return its value
 
diff --git a/tests/test-lock.py b/tests/test-lock.py
--- a/tests/test-lock.py
+++ b/tests/test-lock.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 
 import copy
+import errno
 import os
 import silenttestrunner
 import tempfile
@@ -267,5 +268,30 @@ class testlock(unittest.TestCase):
 
         lock.release()
 
+    def testfrequentlockunlock(self):
+        """This tests whether lock acquisition fails as expected, even if
+        (1) lock can't be acquired (makelock fails by EEXIST), and
+        (2) locker info can't be read in (readlock fails by ENOENT) while
+        retrying 5 times.
+        """
+
+        d = tempfile.mkdtemp(dir=os.getcwd())
+        state = teststate(self, d)
+
+        def emulatefrequentlock(*args):
+            raise OSError(errno.EEXIST, "File exists")
+        def emulatefrequentunlock(*args):
+            raise OSError(errno.ENOENT, "No such file or directory")
+
+        state.vfs.makelock = emulatefrequentlock
+        state.vfs.readlock = emulatefrequentunlock
+
+        try:
+            state.makelock(timeout=0)
+            self.fail("unexpected lock acquisition")
+        except error.LockHeld as why:
+            self.assertTrue(why.errno == errno.ETIMEDOUT)
+            state.assertlockexists(False)
+
 if __name__ == '__main__':
     silenttestrunner.main(__name__)