bug#54062: 29.0.50; [PATCH] Eshell should inform processes when a pipe is broken

[Jim Porter <jporterbugs@gmail.com>]

On 2/19/2022 12:19 PM, Eli Zaretskii wrote:
>> Cc: 54062@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Sat, 19 Feb 2022 12:02:45 -0800
>>
>> One option would be to call `interrupt-process' instead, since that
>> works in all cases I'm aware of. This isn't quite as nice as sending
>> SIGPIPE (or equivalent) to let the process handle it how it wants, but
>> at least `interrupt-process' has the same default behavior as SIGPIPE
>> (i.e. terminate the process).
> 
> Many console programs catch SIGINT, though.
> 
> Can't we terminate ("kill") the process instead?  Or maybe deleting
> the process object is enough?

That might work; it would definitely be better than `interrupt-process'. 
On the other hand, I think it would be nice to handle this case by 
breaking the pipe if possible, since that would be closer to how it 
works in regular shells, as I understand it.

>> Another way would be to add a function like `process-break-pipe' (it
>> could probably use a better name) that would close the read end of the
>> process's output pipe, which - if I understand the Win32 API here -
>> should trigger the right behavior on MS Windows too.
> 
> You mean, delete the process object?  That's how we close our end of
> the pipe, no?

Do you mean using `delete-process'? That works differently from how I'm 
imagining things. From reading the code, `delete-process' sends SIGKILL 
to the process group, but that means that a process that wants to do 
something special in response to SIGPIPE (or EPIPE, or ERROR_BROKEN_PIPE 
on Win32) wouldn't be able to, since that's not the signal/error it 
receives.

In my patch, `process-break-pipe' just closes the file descriptor for 
the read end of the process's stdout pipe, but otherwise doesn't do 
anything to the process. Then, when the process tries to write to stdout 
again, the OS will report (via a signal and/or an error code) that the 
pipe is broken. Since Win32's WriteFile[1] API returns ERROR_BROKEN_PIPE 
in this case, that would let MS Windows programs detect and respond to 
broken pipes in the usual way for that platform.

>> One caveat is that the head process (`yes' in the example), would only
>> see the "broken pipe" error on the *next* write after the one where
>> Eshell detected the broken pipe. That's easy enough to fix for cases
>> where we can signal SIGPIPE directly, but it's probably ok in general
>> too: after all, processes don't generally know exactly when a SIGPIPE
>> might occur, so it occurring slightly later shouldn't cause problems.
> 
> I don't see a problem here.  AFAIU, closing a pipe doesn't always
> deliver SIGPIPE, it can instead fail the write with EPIPE.  So SIGPIPE
> is not guaranteed anyway.

Agreed, I don't think this is really a problem. I just wanted to note 
that the behavior is slightly different from how someone might expect it 
to work in a regular shell. (In any case, I think SIGPIPE and EPIPE 
occur at effectively the same time, and you would check for the latter 
if you ignored SIGPIPE, for example.[2] Maybe this comes with some 
caveats or is specific to glibc though.)

>> (In theory, the tail process should call `process-break-pipe' as soon as
>> it closes, but in Eshell, the tail process doesn't know what's feeding
>> it input, so it can't easily do this.)
> 
> Not sure I understand: an Emacs process object always knows what's
> feeding it.

Emacs process objects could probably do it, but I'm not sure if Eshell's 
pipelines are able to without being reworked. Eshell pipelines are 
assembled pretty indirectly; the output of one process goes through a 
process-filter and into `eshell-output-object', which looks up where to 
send the data in `eshell-current-handles' (which in turn is let-bound so 
each process has its own copy). It would probably take quite a bit of 
work for a process to figure out what's feeding it from its 
process-sentinel, since that happens in a different context than where 
the pipeline is constructed. Maybe it's feasible, but if we agree that 
my caveat in the section above isn't a problem, it would probably be 
simpler to avoid the extra effort.

[1] 
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-writefile
[2] 
https://www.gnu.org/software/libc/manual/html_node/Operation-Error-Signals.html