# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1413512151 25200
# Node ID f484be02bd351fbd084d93f5ba4180e2b22cd4eb
# Parent  3c0983cc279e5c855e4ae13e1a3eb3f47f503da3
lock: while releasing, unlink lockfile even if the release function throws

Consider a hypothetical bug in the release function that causes it to raise an
exception. Also consider the bisect command, which saves its state in a finally
clause. Saving the state requires acquiring the wlock.

If we don't unlink the lockfile when the exception is thrown, we'll try to
acquire the wlock again. We're going to try and acquire a lock again while our
old lockfile is on disk. The PID on disk is our own, and of course we're still
running, so we won't take over the lock. Hence we'll be stuck waiting for a
lock that we left behind ourselves.

To avoid this, always unlink the lockfile. This preserves the invariant that
self.held > 0 is equivalent to the lockfile existing on disk.

diff -r 3c0983cc279e -r f484be02bd35 mercurial/lock.py
--- a/mercurial/lock.py	Fri Oct 17 13:52:10 2014 -0400
+++ b/mercurial/lock.py	Thu Oct 16 19:15:51 2014 -0700
@@ -139,12 +139,14 @@
             if os.getpid() != self.pid:
                 # we forked, and are not the parent
                 return
-            if self.releasefn:
-                self.releasefn()
             try:
-                self.vfs.unlink(self.f)
-            except OSError:
-                pass
+                if self.releasefn:
+                    self.releasefn()
+            finally:
+                try:
+                    self.vfs.unlink(self.f)
+                except OSError:
+                    pass
             for callback in self.postrelease:
                 callback()
 
diff -r 3c0983cc279e -r f484be02bd35 tests/test-lock-badness.t
--- a/tests/test-lock-badness.t	Fri Oct 17 13:52:10 2014 -0400
+++ b/tests/test-lock-badness.t	Thu Oct 16 19:15:51 2014 -0700
@@ -11,6 +11,42 @@
   updating to branch default
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
+Test that raising an exception in the release function doesn't cause the lock to choke
+
+  $ cat > testlock.py << EOF
+  > from mercurial import cmdutil, error, util
+  > 
+  > cmdtable = {}
+  > command = cmdutil.command(cmdtable)
+  > 
+  > def acquiretestlock(repo, releaseexc):
+  >     def unlock():
+  >         if releaseexc:
+  >             raise util.Abort('expected release exception')
+  >     l = repo._lock(repo.vfs, 'testlock', False, unlock, None, 'test lock')
+  >     return l
+  > 
+  > @command('testlockexc')
+  > def testlockexc(ui, repo):
+  >     testlock = acquiretestlock(repo, True)
+  >     try:
+  >         testlock.release()
+  >     finally:
+  >         try:
+  >             testlock = acquiretestlock(repo, False)
+  >         except error.LockHeld:
+  >             raise util.Abort('lockfile on disk even after releasing!')
+  >         testlock.release()
+  > EOF
+  $ cat >> $HGRCPATH << EOF
+  > [extensions]
+  > testlock=$TESTTMP/testlock.py
+  > EOF
+
+  $ hg -R b testlockexc
+  abort: expected release exception
+  [255]
+
 One process waiting for another
 
   $ cat > hooks.py << EOF