evolve: fixed lock acquire before checking state
authorTaapas Agrawal <taapas2897@gmail.com>
Fri, 12 Jul 2019 23:43:25 +0530
changeset 4797 8c780c3eb116
parent 4796 4b1518d69a41
child 4798 2e14a9386316
evolve: fixed lock acquire before checking state Before checking the states `repo.lock()` and `repo.wlock()` need to be acquired. This was not happening in `evolve()` earlier. This patch creates a seperate `_performevolve()` funtion which can be then called with locks acquired. This also removes the redundant lock acuquiring of `abortevolve` and `continueevolve` as lock is now taken earlier.
CHANGELOG
hgext3rd/evolve/evolvecmd.py
--- a/CHANGELOG	Wed Jul 31 10:52:14 2019 -0700
+++ b/CHANGELOG	Fri Jul 12 23:43:25 2019 +0530
@@ -5,6 +5,7 @@
 -------------------
 
   * prune: clarify error message when no revision were passed,
+  * evolve: avoid possible race conditions bu locking earlier
 
 9.1.0 -- 2019-07-29
 -------------------
--- a/hgext3rd/evolve/evolvecmd.py	Wed Jul 31 10:52:14 2019 -0700
+++ b/hgext3rd/evolve/evolvecmd.py	Fri Jul 12 23:43:25 2019 +0530
@@ -1607,7 +1607,10 @@
          aborts the interrupted evolve and undoes all the resolution which have
          happened
     """
+    with repo.wlock(), repo.lock():
+        return _performevolve(ui, repo, **opts)
 
+def _performevolve(ui, repo, **opts):
     opts = _checkevolveopts(repo, opts)
     # Options
     contopt = opts['continue']
@@ -1722,15 +1725,14 @@
         lastsolved = None
 
         activetopic = getattr(repo, 'currenttopic', '')
-        with repo.wlock(), repo.lock():
-            tr = repo.transaction("evolve")
-            with util.acceptintervention(tr):
-                for rev in revs:
-                    lastsolved = _solveonerev(ui, repo, rev, evolvestate,
-                                              activetopic, dryrunopt,
-                                              confirmopt, progresscb,
-                                              targetcat, lastsolved)
-                    seen += 1
+        tr = repo.transaction("evolve")
+        with util.acceptintervention(tr):
+            for rev in revs:
+                lastsolved = _solveonerev(ui, repo, rev, evolvestate,
+                                          activetopic, dryrunopt,
+                                          confirmopt, progresscb,
+                                          targetcat, lastsolved)
+                seen += 1
 
         if showprogress:
             compat.progress(ui, _('evolve'), None)
@@ -1857,123 +1859,122 @@
                          hint=_("see 'hg help phases' for details"))
             cleanup = False
 
-        # checking no new changesets are created on evolved revs
-        descendants = set()
-        if evolvedrevs:
-            descendants = set(repo.changelog.descendants(evolvedrevs))
-        if descendants - set(evolvedrevs):
-            repo.ui.warn(_("warning: new changesets detected on destination "
-                           "branch\n"))
-            cleanup = False
+    # checking no new changesets are created on evolved revs
+    descendants = set()
+    if evolvedrevs:
+        descendants = set(repo.changelog.descendants(evolvedrevs))
+    if descendants - set(evolvedrevs):
+        repo.ui.warn(_("warning: new changesets detected on destination "
+                       "branch\n"))
+        cleanup = False
 
-        # finding the indices of the obsmarkers to be stripped and stripping
-        # them
-        if evolvestate['obsmarkers']:
-            stripmarkers = set()
-            for m in evolvestate['obsmarkers']:
-                m = (m[0], m[1])
-                stripmarkers.add(m)
-            indices = []
-            allmarkers = obsutil.getmarkers(repo)
-            for i, m in enumerate(allmarkers):
-                marker = (m.prednode(), m.succnodes()[0])
-                if marker in stripmarkers:
-                    indices.append(i)
+    # finding the indices of the obsmarkers to be stripped and stripping
+    # them
+    if evolvestate['obsmarkers']:
+        stripmarkers = set()
+        for m in evolvestate['obsmarkers']:
+            m = (m[0], m[1])
+            stripmarkers.add(m)
+        indices = []
+        allmarkers = obsutil.getmarkers(repo)
+        for i, m in enumerate(allmarkers):
+            marker = (m.prednode(), m.succnodes()[0])
+            if marker in stripmarkers:
+                indices.append(i)
 
-            repair.deleteobsmarkers(repo.obsstore, indices)
-            repo.ui.debug('deleted %d obsmarkers\n' % len(indices))
+        repair.deleteobsmarkers(repo.obsstore, indices)
+        repo.ui.debug('deleted %d obsmarkers\n' % len(indices))
 
-        if cleanup:
-            if evolvedrevs:
-                strippoints = [c.node()
-                               for c in repo.set('roots(%ld)', evolvedrevs)]
+    if cleanup:
+        if evolvedrevs:
+            strippoints = [c.node()
+                           for c in repo.set('roots(%ld)', evolvedrevs)]
 
-            # updating the working directory
-            hg.updaterepo(repo, startnode, True)
+        # updating the working directory
+        hg.updaterepo(repo, startnode, True)
 
-            # Strip from the first evolved revision
-            if evolvedrevs:
-                # no backup of evolved cset versions needed
-                repair.strip(repo.ui, repo, strippoints, False)
+        # Strip from the first evolved revision
+        if evolvedrevs:
+            # no backup of evolved cset versions needed
+            repair.strip(repo.ui, repo, strippoints, False)
 
-            with repo.transaction('evolve') as tr:
-                # restoring bookmarks at there original place
-                bmchanges = evolvestate['bookmarkchanges']
-                if bmchanges:
-                    repo._bookmarks.applychanges(repo, tr, bmchanges)
+        with repo.transaction('evolve') as tr:
+            # restoring bookmarks at there original place
+            bmchanges = evolvestate['bookmarkchanges']
+            if bmchanges:
+                repo._bookmarks.applychanges(repo, tr, bmchanges)
 
-            evolvestate.delete()
-            ui.status(_('evolve aborted\n'))
-            ui.status(_('working directory is now at %s\n')
-                      % nodemod.hex(startnode)[:12])
-        else:
-            raise error.Abort(_("unable to abort interrupted evolve, use 'hg "
-                                "evolve --stop' to stop evolve"))
+        evolvestate.delete()
+        ui.status(_('evolve aborted\n'))
+        ui.status(_('working directory is now at %s\n')
+                  % nodemod.hex(startnode)[:12])
+    else:
+        raise error.Abort(_("unable to abort interrupted evolve, use 'hg "
+                            "evolve --stop' to stop evolve"))
 
 def continueevolve(ui, repo, evolvestate):
     """logic for handling of `hg evolve --continue`"""
 
-    with repo.wlock(), repo.lock():
-        ms = merge.mergestate.read(repo)
-        mergeutil.checkunresolved(ms)
-        if (evolvestate['command'] == 'next'
-            or evolvestate['category'] == 'orphan'):
-            _completeorphan(ui, repo, evolvestate)
-        elif evolvestate['category'] == 'phasedivergent':
-            _completephasedivergent(ui, repo, evolvestate)
-        elif evolvestate['category'] == 'contentdivergent':
-            _continuecontentdivergent(ui, repo, evolvestate, None)
-        else:
-            repo.ui.status(_("continuing interrupted '%s' resolution is not yet"
-                             " supported\n") % evolvestate['category'])
-            return
+    ms = merge.mergestate.read(repo)
+    mergeutil.checkunresolved(ms)
+    if (evolvestate['command'] == 'next'
+        or evolvestate['category'] == 'orphan'):
+        _completeorphan(ui, repo, evolvestate)
+    elif evolvestate['category'] == 'phasedivergent':
+        _completephasedivergent(ui, repo, evolvestate)
+    elif evolvestate['category'] == 'contentdivergent':
+        _continuecontentdivergent(ui, repo, evolvestate, None)
+    else:
+        repo.ui.status(_("continuing interrupted '%s' resolution is not yet"
+                         " supported\n") % evolvestate['category'])
+        return
 
-        # make sure we are continuing evolve and not `hg next --evolve`
-        if evolvestate['command'] != 'evolve':
-            return
+    # make sure we are continuing evolve and not `hg next --evolve`
+    if evolvestate['command'] != 'evolve':
+        return
 
-        # Progress handling
-        seen = 1
-        count = len(evolvestate['revs'])
+    # Progress handling
+    seen = 1
+    count = len(evolvestate['revs'])
 
-        def progresscb():
-            compat.progress(ui, _('evolve'), seen, unit=_('changesets'),
-                            total=count)
+    def progresscb():
+        compat.progress(ui, _('evolve'), seen, unit=_('changesets'),
+                        total=count)
 
-        category = evolvestate['category']
-        confirm = evolvestate['confirm']
-        unfi = repo.unfiltered()
-        # lastsolved: keep track of successor of last troubled cset we
-        # evolved to confirm that if atop msg should be suppressed to remove
-        # redundancy
-        lastsolved = None
-        activetopic = getattr(repo, 'currenttopic', '')
-        tr = repo.transaction("evolve")
-        with util.acceptintervention(tr):
-            for rev in evolvestate['revs']:
-                # XXX: prevent this lookup by storing nodes instead of revnums
-                curctx = unfi[rev]
+    category = evolvestate['category']
+    confirm = evolvestate['confirm']
+    unfi = repo.unfiltered()
+    # lastsolved: keep track of successor of last troubled cset we
+    # evolved to confirm that if atop msg should be suppressed to remove
+    # redundancy
+    lastsolved = None
+    activetopic = getattr(repo, 'currenttopic', '')
+    tr = repo.transaction("evolve")
+    with util.acceptintervention(tr):
+        for rev in evolvestate['revs']:
+            # XXX: prevent this lookup by storing nodes instead of revnums
+            curctx = unfi[rev]
 
-                # check if we can use stack template
-                revtopic = getattr(curctx, 'topic', lambda: '')()
-                topicidx = getattr(curctx, 'topicidx', lambda: None)()
-                stacktmplt = False
-                if (activetopic and (activetopic == revtopic)
-                    and topicidx is not None):
-                    stacktmplt = True
+            # check if we can use stack template
+            revtopic = getattr(curctx, 'topic', lambda: '')()
+            topicidx = getattr(curctx, 'topicidx', lambda: None)()
+            stacktmplt = False
+            if (activetopic and (activetopic == revtopic)
+                and topicidx is not None):
+                stacktmplt = True
 
-                if (curctx.node() not in evolvestate['replacements']
-                    and curctx.node() not in evolvestate['skippedrevs']):
-                    newnode = _solveone(ui, repo, curctx, evolvestate, False,
-                                        confirm, progresscb, category,
-                                        lastsolved=lastsolved,
-                                        stacktmplt=stacktmplt)
-                    if newnode[0]:
-                        evolvestate['replacements'][curctx.node()] = newnode[1]
-                        lastsolved = newnode[1]
-                    else:
-                        evolvestate['skippedrevs'].append(curctx.node())
-                seen += 1
+            if (curctx.node() not in evolvestate['replacements']
+                and curctx.node() not in evolvestate['skippedrevs']):
+                newnode = _solveone(ui, repo, curctx, evolvestate, False,
+                                    confirm, progresscb, category,
+                                    lastsolved=lastsolved,
+                                    stacktmplt=stacktmplt)
+                if newnode[0]:
+                    evolvestate['replacements'][curctx.node()] = newnode[1]
+                    lastsolved = newnode[1]
+                else:
+                    evolvestate['skippedrevs'].append(curctx.node())
+            seen += 1
 
 def _continuecontentdivergent(ui, repo, evolvestate, progresscb):
     """function to continue the interrupted content-divergence resolution."""