[PATCH] dispatch: treat SIGPIPE as a termination signal (BC)

Simon Farnsworth simonfar at fb.com
Tue Feb 7 15:28:04 EST 2017


On 07/02/2017 20:17, Jun Wu wrote:
> This approach looks good to me. The only problem is it will print "killed!"
> on SIGPIPE so maybe a follow-up to make it silent.
>

Is there a practical case where "killed!" on stderr when the command is 
aborted due to SIGPIPE is a bad thing?

In the pager case, this is only visible if stderr is redirected, but 
stdout is not - if stderr isn't redirected, then the pager (which has 
just exited - otherwise we wouldn't be getting SIGPIPE) will be sent the 
string, and won't print it.

In other cases, I think the extra message is desirable (and justifies 
the BC marker). It tells you that where old Mercurial would continue 
processing after your stdin feed or stdout capture died unexpectedly, 
this Mercurial now quits.

> Excerpts from Simon Farnsworth's message of 2017-02-07 12:08:29 -0800:
>> # HG changeset patch
>> # User Simon Farnsworth <simonfar at fb.com>
>> # Date 1486498104 28800
>> #      Tue Feb 07 12:08:24 2017 -0800
>> # Node ID e1150763706818fffa62c81a72a88574d20caea1
>> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
>> dispatch: treat SIGPIPE as a termination signal (BC)
>>
>> pager previously set SIGPIPE to immediately exit the process, ignoring any
>> Python @atexit handlers, exception handling etc - just instant termination.
>>
>> Simply removing this as per changeset aaa751585325 meant that when you
>> aborted a long running pager command, Mercurial would not quit; instead, we
>> should add SIGPIPE to the list of termination signals in dispatch.py.
>>
>> This is a slight BC break, as previously, a process that was piping data
>> into or out of Mercurial would not kill Mercurial if it died before closing
>> its end of the pipe, whereas it will now cause Mercurial to exit.
>>
>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
>> --- a/mercurial/dispatch.py
>> +++ b/mercurial/dispatch.py
>> @@ -147,7 +147,7 @@
>>
>>      ui = req.ui
>>      try:
>> -        for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM':
>> +        for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM', 'SIGPIPE':
>>              num = getattr(signal, name, None)
>>              if num:
>>                  signal.signal(num, catchterm)

-- 
Simon Farnsworth


More information about the Mercurial-devel mailing list