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

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2751 bytes --]

On 2/21/2022 10:31 AM, Eli Zaretskii wrote:
>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: Jim Porter <jporterbugs@gmail.com>,  54062@debbugs.gnu.org
>> Date: Mon, 21 Feb 2022 18:39:16 +0100
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> Thanks; I have no further comments.
>>
>> So I've pushed the patch series to Emacs 29.
> 
> Something's amiss here: the new test fails on MS-Windows because it
> signals an error inside eshell-wait-for-subprocess:
> 
>    Test esh-proc-test/sigpipe-exits-process backtrace:
>      signal(eshell-pipe-broken #<process sh.exe>)
>      eshell-output-object-to-target("killed\n" #<process sh.exe>)
>      eshell-output-object("killed\n" nil [nil (nil . 0) (nil . 0)])
>      #f(compiled-function () #<bytecode 0x1c2b272f8d5f2c5e>)()
>      apply(#f(compiled-function () #<bytecode 0x1c2b272f8d5f2c5e>) nil)
>      timer-event-handler([t 25107 54636 874750 nil #f(compiled-function (
>      sleep-for(0.1)
>      sit-for(0.1)
>      (while (if all eshell-process-list (eshell-interactive-process-p)) (
>      (let ((start (current-time))) (while (if all eshell-process-list (es
>      eshell-wait-for-subprocess(t)
> 
> Sounds like the shell is already dead/killed when
> eshell-wait-for-subprocess tries to send it a string?

Thanks for merging, and sorry about the bustage. This turned out to be 
because `eshell-sentinel' for the "head" process in the pipeline called 
`eshell-output-object', but because the tail process was already dead, 
it raised `eshell-pipe-broken'. I believe the reason this only 
manifested on MS Windows was due to a timing difference between 
`delete-process' and `signal-process'; using the `delete-process' path 
on GNU/Linux shows the same problem.

Attached is a patch that ignores the `eshell-pipe-broken' error in 
`eshell-sentinel'. It's not really an error in that case anyway, since 
we only want to write the last bit of output *if we can*.

--------------------

There's just one problem remaining: when running the following command 
on MS Windows[1], you'll (usually) see two Eshell prompts get emitted 
after it finishes:

   yes | sh -c 'read NAME'

However, this is a separate bug that appears in Emacs 27.2 as well. It 
can happen whenever multiple commands in a pipeline get killed. For example:

   ~ $ sh -c 'while true; do sleep 1; echo y; done' | sh -c 'while true; 
do read NAME; echo ${NAME}; done'
   C-c C-c  ; Call `eshell-interrupt-process'

The same happens with C-c C-k (`eshell-kill-process') too. I'll file 
another bug about this, but I wanted to mention it here so no one's 
surprised if they see this come up when testing out this patch.

[1] Well, I assume it's a problem on MS Windows. I actually tested the 
MS Windows code path on GNU/Linux.

[-- Attachment #2: 0001-Ignore-eshell-broken-pipe-error-in-eshell-sentinel.patch --]
[-- Type: text/plain, Size: 1805 bytes --]

From f41b9e2599fe3a1a959ef1b52f77032fd8d28c20 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 21 Feb 2022 12:14:34 -0800
Subject: [PATCH] Ignore 'eshell-broken-pipe' error in 'eshell-sentinel'

This can happen if 'eshell-sentinel' tries to write output to another
process, but that process has already terminated.

* lisp/eshell/esh-proc.el (eshell-sentinel): Use 'ignore-error'
instead of 'unwind-protect'.
---
 lisp/eshell/esh-proc.el | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index ed37de85f7..d7d22d2a9e 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -435,12 +435,12 @@ eshell-sentinel
                                   (lambda ()
                                     (if (nth 4 entry)
                                         (run-at-time 0 nil finish-io)
-                                      (unwind-protect
-                                          (when str
-                                            (eshell-output-object
-                                             str nil handles))
-                                        (eshell-close-handles
-                                         status 'nil handles))))))
+                                      (when str
+                                        (ignore-error 'eshell-pipe-broken
+                                          (eshell-output-object
+                                           str nil handles)))
+                                      (eshell-close-handles
+                                       status 'nil handles)))))
                           (funcall finish-io)))))
 		(eshell-remove-process-entry entry))))
 	(eshell-kill-process-function proc string)))))
-- 
2.25.1