[PATCH STABLE] extensions: clear aftercallbacks after execution (issue4646)

[Pierre-Yves David]

On 05/05/2015 10:39 PM, Yuya Nishihara wrote:
> On Tue, 05 May 2015 21:35:25 -0700, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1430886916 25200
>> #      Tue May 05 21:35:16 2015 -0700
>> # Branch stable
>> # Node ID 4da7e74074010404744171ad68c1a1bd7d100696
>> # Parent  41cd8171e58f991373dcd0b4897dc1e5978d42dd
>> extensions: clear aftercallbacks after execution (issue4646)
>>
>> It was reported that enabling pager without color could cause a hang.
>> Inserting print statements revealed that the callbacks in
>> extensions._aftercallbacks were being invoked twice.
>>
>> I'm not sure exactly how the hang comes to be. But, clearing
>> extensions._aftercallbacks makes the problem go away. This presumably
>> has something to do with forked processes inheriting
>> extensions._aftercallbacks and that somehow results in badness.
>>
>> The reproduce steps in the bug seem to only occur when the output of
>> a command is less than the size of the current screen. This is not
>> something that can easily be tested. I verified the test case works
>> with this patch and that pager and color interaction continues to
>> work. Since we have no existing automated tests for pager, this sadly
>> appears to be the best I can do.
>>
>> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
>> --- a/mercurial/extensions.py
>> +++ b/mercurial/extensions.py
>> @@ -133,8 +133,12 @@ def loadall(ui):
>>
>>           for fn in _aftercallbacks[shortname]:
>>               fn(loaded=False)
>>
>> +    # Forked processes appear to inherit _aftercallbacks, which may cause
>> +    # deadlock. See issue4646.
>> +    _aftercallbacks.clear()
>
> I don't think this is related to fork(). extensions.loadall() is just called
> more than once.
>
> % grep extensions.loadall mercurial/*.py
> mercurial/dispatch.py:    extensions.loadall(lui)
> mercurial/dispatch.py:    # (uisetup and extsetup are handled in extensions.loadall)
> mercurial/localrepo.py:            extensions.loadall(self.ui)

We probably call it (1) at the very beginning to load global/user 
enabled extension (that could be defining new command or alter the repo 
creation process). (2) after a repo is created and its UI is parsed.

-- 
Pierre-Yves David