view tests/test-mq-qclone-http.t @ 43709:039fbd14d4e2

lock: fix race in lock-breaking code

With low frequency, I see hg pulls fail with output like:
abort: no such file or directory: .hg/store/lock

I think what happens is, in lock.py, in:

    def _testlock(self, locker):
        if not self._lockshouldbebroken(locker):
            return locker

        # if locker dead, break lock.  must do this with another lock
        # held, or can race and break valid lock.
        try:
            with lock(self.vfs, self.f + b'.break', timeout=0):
                self.vfs.unlink(self.f)
        except error.LockError:
            return locker

if a lock is breakable on disk, and two hg processes concurrently get
to the "if locker dead" comment, a possible interleaving is: process1
finishes executing the function and then process2 finishes executing
the function. If that happens, process2 will either get ENOENT in
self.vfs.unlink (resulting in the spurious failure above), or break a
valid lock and potentially cause repository corruption.

The fix is simple enough: make sure the lock is breakable _inside_ the
critical section, because only then can we know that no other process
can invalidate our knowledge on the lock on disk.

I don't think there are tests for this. I've tested this manually
with:

diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -351,6 +351,8 @@ class lock(object):
         if not self._lockshouldbebroken(locker):
             return locker

+        import random
+        time.sleep(1. + random.random())
         # if locker dead, break lock.  must do this with another lock
         # held, or can race and break valid lock.
         try:
@@ -358,6 +360,7 @@ class lock(object):
                 self.vfs.unlink(self.f)
         except error.LockError:
             return locker
+        time.sleep(1)

     def testlock(self):
         """return id of locker if lock is valid, else None.

and I see this change of behavior before/after this commit:

 $ $hg init repo
 $ cd repo
 $ ln -s $HOSTNAME/effffffc:987654321 .hg/wlock
 $ touch a
 $ $hg commit -Am_ & $hg commit -Am _; wait
-abort: No such file or directory: '/tmp/repo/.hg/wlock'
 adding a
+warning: ignoring unknown working parent 679a8959a8ca!
+nothing changed

Differential Revision: https://phab.mercurial-scm.org/D7199

hide outer repo
  $ hg init

  $ echo "[extensions]" >> $HGRCPATH
  $ echo "mq=" >> $HGRCPATH
  $ mkdir webdir
  $ cd webdir
  $ hg init a
  $ hg --cwd a qinit -c
  $ echo a > a/a
  $ hg --cwd a ci -A -m a
  adding a
  $ echo b > a/b
  $ hg --cwd a addremove
  adding b
  $ hg --cwd a qnew -f b.patch
  $ hg --cwd a qcommit -m b.patch
  $ hg --cwd a log --template "{desc}\n"
  [mq]: b.patch
  a
  $ hg --cwd a/.hg/patches log --template "{desc}\n"
  b.patch
  $ root=`pwd`
  $ cd ..

test with recursive collection

  $ cat > collections.conf <<EOF
  > [paths]
  > /=$root/**
  > EOF
  $ hg serve -p $HGPORT -d --pid-file=hg.pid --webdir-conf collections.conf \
  >     -A access-paths.log -E error-paths-1.log
  $ cat hg.pid >> $DAEMON_PIDS
  $ get-with-headers.py localhost:$HGPORT '?style=raw'
  200 Script output follows
  
  
  /a/
  /a/.hg/patches/
  
  $ hg qclone http://localhost:$HGPORT/a b
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 2 changesets with 2 changes to 2 files
  new changesets cb9a9f314b8b:184916345baa
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 1 changesets with 3 changes to 3 files
  new changesets 4052ceaa8c4e
  updating to branch default
  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
  $ hg --cwd b log --template "{desc}\n"
  a
  $ hg --cwd b qpush -a
  applying b.patch
  now at: b.patch
  $ hg --cwd b log --template "{desc}\n"
  imported patch b.patch
  a

test with normal collection

  $ cat > collections1.conf <<EOF
  > [paths]
  > /=$root/*
  > EOF
  $ hg serve -p $HGPORT1 -d --pid-file=hg.pid --webdir-conf collections1.conf \
  >     -A access-paths.log -E error-paths-1.log
  $ cat hg.pid >> $DAEMON_PIDS
  $ get-with-headers.py localhost:$HGPORT1 '?style=raw'
  200 Script output follows
  
  
  /a/
  /a/.hg/patches/
  
  $ hg qclone http://localhost:$HGPORT1/a c
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 2 changesets with 2 changes to 2 files
  new changesets cb9a9f314b8b:184916345baa
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 1 changesets with 3 changes to 3 files
  new changesets 4052ceaa8c4e
  updating to branch default
  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
  $ hg --cwd c log --template "{desc}\n"
  a
  $ hg --cwd c qpush -a
  applying b.patch
  now at: b.patch
  $ hg --cwd c log --template "{desc}\n"
  imported patch b.patch
  a

test with old-style collection

  $ cat > collections2.conf <<EOF
  > [collections]
  > $root=$root
  > EOF
  $ hg serve -p $HGPORT2 -d --pid-file=hg.pid --webdir-conf collections2.conf \
  >     -A access-paths.log -E error-paths-1.log
  $ cat hg.pid >> $DAEMON_PIDS
  $ get-with-headers.py localhost:$HGPORT2 '?style=raw'
  200 Script output follows
  
  
  /a/
  /a/.hg/patches/
  
  $ hg qclone http://localhost:$HGPORT2/a d
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 2 changesets with 2 changes to 2 files
  new changesets cb9a9f314b8b:184916345baa
  requesting all changes
  adding changesets
  adding manifests
  adding file changes
  added 1 changesets with 3 changes to 3 files
  new changesets 4052ceaa8c4e
  updating to branch default
  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
  $ hg --cwd d log --template "{desc}\n"
  a
  $ hg --cwd d qpush -a
  applying b.patch
  now at: b.patch
  $ hg --cwd d log --template "{desc}\n"
  imported patch b.patch
  a

test --mq works and uses correct repository config

  $ hg --cwd d outgoing --mq
  comparing with http://localhost:$HGPORT2/a/.hg/patches
  searching for changes
  no changes found
  [1]
  $ hg --cwd d log --mq --template '{rev} {desc|firstline}\n'
  0 b.patch

  $ killdaemons.py