Patchwork [Bug,4368] New: Parallel committing results in orphaned commits

login
register
mail settings
Submitter mercurial-bugs@selenic.com
Date Sept. 12, 2014, 8:26 p.m.
Message ID <bug-4368-285@http.bz.selenic.com/>
Download mbox | patch
Permalink /patch/5810/
State Not Applicable
Headers show

Comments

mercurial-bugs@selenic.com - Sept. 12, 2014, 8:26 p.m.
http://bz.selenic.com/show_bug.cgi?id=4368

          Priority: normal
            Bug ID: 4368
                CC: mercurial-devel@selenic.com
          Assignee: bugzilla@selenic.com
           Summary: Parallel committing results in orphaned commits
          Severity: bug
    Classification: Unclassified
                OS: Mac OS
          Reporter: durham@fb.com
          Hardware: PC
            Status: UNCONFIRMED
           Version: 3.1.1
         Component: Mercurial
           Product: Mercurial

A script such as:

#!/bin/bash
touch sequence
hg add sequence
for i in {1..10}; do
  ( echo $i >> sequence; hg commit -m "commit $i" ) &
done

Can result in commits with nullid as the parent:

~/local/foo> hg log -G
@  changeset:   2:815cf99f041e
   parent:      -1:000000000000
   summary:     commit 5

o  changeset:   1:f4d3097793d2
|  summary:     commit 3
|
o  changeset:   0:bc561cb88245
   summary:     a

This is caused by localrepo.commit taking the wlock but not the lock. This
result in localrepo.commit reading the new dirstate, but not the new changelog,
so when it builds the wctx to be committed, to dirstate parent is missing and
it uses nullid instead.  Resulting in an orphaned commit.

Possible fixes:
1) invalidate the changelog when the wlock is taken:
Or probably do both #1 and #2.  Even with those fixes, the script still
occasionally prints out 'created new head' despite no new heads being created. 
I think this is probably an issue with the branch head cache not being
invalidate after taking the lock.  or something.

Patch

--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1112,7 +1113,7 @@ 
             self._filecache['dirstate'].refresh()

         l = self._lock(self.vfs, "wlock", wait, unlock,
-                       self.invalidatedirstate, _('working directory of %s') %
+                       self.invalidate, _('working directory of %s') %
                        self.origroot)
         self._wlockref = weakref.ref(l)
         return l

2) take the real lock at the same time as the wlock
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -475,6 +475,7 @@ 

     # the backout should appear on the same branch
     wlock = repo.wlock()
+    lock = repo.lock()
     try:
         branch = repo.dirstate.branch()
         bheads = repo.branchheads(branch)
@@ -535,6 +536,7 @@ 
             finally:
                 ui.setconfig('ui', 'forcemerge', '', '')
     finally:
+        lock.release()
         wlock.release()
     return 0