Patchwork [STABLE] pull: avoid race condition with 'hg pull --rev name --update' (issue4706)

login
register
mail settings
Submitter Pierre-Yves David
Date June 3, 2015, 9:54 p.m.
Message ID <61c8ae4548b0d1d5c8b6.1433368452@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/9469/
State Accepted
Headers show

Comments

Pierre-Yves David - June 3, 2015, 9:54 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1433366951 25200
#      Wed Jun 03 14:29:11 2015 -0700
# Branch stable
# Node ID 61c8ae4548b0d1d5c8b63a6567800e738582b859
# Parent  78e8890cfb4b49fad98f6cd5419fd736fb24aec0
pull: avoid race condition with 'hg pull --rev name --update' (issue4706)

The previous scheme was:

1) lookup node for all pulled revision,
2) pull said node
3) lookup the node of the checkout target
4) update the repository there.

If the remote repo changes between (1) and (3), the resolved name will be
different and (3) crash. There is actually no need for a remote lookup during
(3), we could just set the value in (1). This prevent the race condition and
save a possible network roundtrip.
Matt Mackall - June 3, 2015, 10:10 p.m.
On Wed, 2015-06-03 at 14:54 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1433366951 25200
> #      Wed Jun 03 14:29:11 2015 -0700
> # Branch stable
> # Node ID 61c8ae4548b0d1d5c8b63a6567800e738582b859
> # Parent  78e8890cfb4b49fad98f6cd5419fd736fb24aec0
> pull: avoid race condition with 'hg pull --rev name --update' (issue4706)

Queued for stable, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5110,21 +5110,27 @@  def pull(ui, repo, source="default", **o
                     raise util.Abort(_('remote bookmark %s not found!') % b)
                 revs.append(remotebookmarks[b])
 
         if revs:
             try:
-                revs = [other.lookup(rev) for rev in revs]
+                oldrevs = revs
+                revs = [] # actually, nodes
+                for r in oldrevs:
+                    node = other.lookup(r)
+                    revs.append(node)
+                    if r == checkout:
+                        checkout = node
             except error.CapabilityError:
                 err = _("other repository doesn't support revision lookup, "
                         "so a rev cannot be specified.")
                 raise util.Abort(err)
 
         modheads = exchange.pull(repo, other, heads=revs,
                                  force=opts.get('force'),
                                  bookmarks=opts.get('bookmark', ())).cgresult
         if checkout:
-            checkout = str(repo.changelog.rev(other.lookup(checkout)))
+            checkout = str(repo.changelog.rev(checkout))
         repo._subtoppath = source
         try:
             ret = postincoming(ui, repo, modheads, opts.get('update'), checkout)
 
         finally:
diff --git a/tests/test-pull-r.t b/tests/test-pull-r.t
--- a/tests/test-pull-r.t
+++ b/tests/test-pull-r.t
@@ -99,6 +99,46 @@  Pull multiple revisions with update:
 
 This used to abort: received changelog group is empty:
 
   $ hg pull -qr 1 ../repo
 
+Test race condition with -r and -U (issue4707)
+
+We pull '-U -r <name>' and the name change right after/during the changegroup emission.
+We use http because http is better is our racy-est option.
+
+
+  $ echo babar > ../repo/jungle
+  $ cat <<EOF > ../repo/.hg/hgrc
+  > [hooks]
+  > outgoing.makecommit = hg ci -Am 'racy commit'; echo committed in pull-race
+  > EOF
+  $ hg -R ../repo serve -p $HGPORT2 -d --pid-file=../repo.pid
+  $ cat ../repo.pid >> $DAEMON_PIDS
+  $ hg pull --rev default --update http://localhost:$HGPORT2/
+  pulling from http://localhost:$HGPORT2/
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files (+1 heads)
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg log -G
+  @  changeset:   2:effea6de0384
+  |  tag:         tip
+  |  parent:      0:bbd179dfa0a7
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     add bar
+  |
+  | o  changeset:   1:ed1b79f46b9a
+  |/   user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     change foo
+  |
+  o  changeset:   0:bbd179dfa0a7
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     add foo
+  
+
   $ cd ..