bug#72117: Command doesn't execute correctly in eshell

[Jim Porter <jporterbugs@gmail.com>]

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

On 7/14/2024 6:01 PM, Jim Porter wrote:
> On 7/14/2024 12:33 PM, the_wurfkreuz via Bug reports for GNU Emacs, the 
> Swiss army knife of text editors wrote:
>   But that's not the most interesting part. I can execute the
>> command with 'eshell/sudo'. But after the command execution emacs will
>> take the whole CPU processing time if i have 
>> '(global-display-line-numbers-mode 1)'
>> option enabled.
> 
> I can't reproduce this particular issue, but I can reproduce *an* issue. 
> It seems to be due to "head -10" exiting early (as it should), which 
> results in a broken pipe. However, Eshell's broken pipe handling wasn't 
> properly cleaning up the process ("grep" in this example).

Eli, would this patch (re-attached here) be ok for Emacs 30? It's not a 
perfect solution (the fix currently on master is much better), but it's 
the least-risky change to this code that I could make, and it should 
improve matters somewhat for the release branch.

To summarize the problem so you have a bit more context for what this is 
changing: in Eshell, if you have a pipeline like "cat some-file | head 
-10", then the "head" process can exit early and raise an 
'eshell-pipe-broken' signal. The process filter for "cat" will catch 
this signal and try to send SIGPIPE to the actual "cat" process. 
However, for MS-Windows (which doesn't have SIGPIPE), and Tramp (where I 
couldn't get 'signal-process' to work due to another bug[1]), it just 
deletes the process. Unfortunately, that was failing to clean up the 
stderr "process" object, which caused Eshell to think the command wasn't 
complete. By changing to 'kill-process', this cleanup happens and things 
work more smoothly.

If you'd prefer to leave it as-is on Emacs 30, I don't mind either. This 
bug has been present since 29.1, and it took quite a while for anyone to 
notice it, so it probably doesn't come up terribly often.

[1] This is now fixed.

[-- Attachment #2: 0001-Use-kill-process-as-a-fallback-when-a-pipe-gets-brok.patch --]
[-- Type: text/plain, Size: 1715 bytes --]

From cc188c56e0b4313e55af60683f2d1cf9c7f00e1b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 14 Jul 2024 16:51:43 -0700
Subject: [PATCH] Use 'kill-process' as a fallback when a pipe gets broken in
 Eshell

This is better than 'delete-process' since it will ensure that any
stderr pipe-processes get stopped as well (bug#72117).

* lisp/eshell/esh-proc.el (eshell-insertion-filter): Use 'kill-process'
instead of 'delete-process'.
---
 lisp/eshell/esh-proc.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index 0dcdf3bb76c..cb57bb305cc 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -519,7 +519,7 @@ eshell-insertion-filter
                   ;; here.  However, remote processes don't currently
                   ;; support that, and not all systems have SIGPIPE in
                   ;; the first place (e.g. MS Windows).  In these
-                  ;; cases, just delete the process; this is
+                  ;; cases, just kill the process; this is
                   ;; reasonably close to the right behavior, since the
                   ;; default action for SIGPIPE is to terminate the
                   ;; process.  For use cases where SIGPIPE is truly
@@ -529,7 +529,7 @@ eshell-insertion-filter
                   (eshell-pipe-broken
                    (if (or (process-get proc 'remote-pid)
                            (eq system-type 'windows-nt))
-                       (delete-process proc)
+                       (kill-process proc)
                      (signal-process proc 'SIGPIPE))))))
                 (process-put proc :eshell-busy nil))))))
 
-- 
2.25.1