# HG changeset patch # User Taapas Agrawal # Date 1562955205 -19800 # Node ID 8c780c3eb11676e162d900d3a40b6adc77384519 # Parent 4b1518d69a416eee0f159ccdd7bc9740a33c92be 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. diff -r 4b1518d69a41 -r 8c780c3eb116 CHANGELOG --- 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 ------------------- diff -r 4b1518d69a41 -r 8c780c3eb116 hgext3rd/evolve/evolvecmd.py --- 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."""