Subject: Re: bug#79079: 31.0.50; Piped command output is sometimes lost in Eshell

From: Jim Porter <[email-masked]>
Date: Sat, 26 Jul 2025 14:12:59 -0700

On 7/26/2025 10:23 AM, Jim Porter wrote:
> On 7/26/2025 3:15 AM, Eli Zaretskii wrote:
>>> Date: Thu, 24 Jul 2025 08:52:16 -0700
>>> Cc: [email-masked], [email-masked]
>>> From: Jim Porter <[email-masked]>
>>>
>> So calling the filter of the "head"s process when we get EPIPE should
>> fix that, no?
> 
> It would probably fix this particular case, but I'm not sure it's the 
> correct way to do it in general. I'll construct some test programs to 
> test out some of the corner cases here so that we can be sure whatever 
> implementation we do settle on works correctly in all cases.

After examining this more closely, there are three problems with the 
current Emacs code, of varying importance:

1. When we receive an EPIPE, we don't call our process-filter one last 
time (this is the original bug report). You can see this by running the 
following Eshell command repeatedly until it produces no output:

   git log | head -n 10 | cat

2. When we receive an EPIPE, we set the exit status to 256; that's not 
right, since the exit status should be 0. You can see this by running 
the following Eshell command until the prompt shows a non-zero status:

   git log | head -n 10 > #<my-buffer>

3. If the child process closes stdin and wants to keep doing some work 
(like write to stdout), it can't, since we called 'deactivate_process'. 
You can see this with the attached script by running (it's supposed to 
print the first line of "git log"):

   git log | ./test-script.sh

I've also attached two WIP patches for the different implementations. 
"bad-call-filter.diff" uses 'read_process_output' to call our process 
filter. It only solves issue #1 above. "good-close-pipe.diff" (which I 
posted previously) solves all three issues. While the latter is a 
slightly bigger change and requires some more testing on my end to make 
sure I've covered 100% of the cases, I think it's the more-correct 
solution. If we make the bigger change, it's probably worth calling it 
out in NEWS and on emacs-devel so that people can be on the lookout for 
any edge cases I haven't identified.

diff --git a/src/process.c b/src/process.c
index e61ec425f7e..150f1dff1c9 100644
--- a/src/process.c
+++ b/src/process.c
@@ -6873,9 +6873,13 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len,
 		}
 	      else if (errno == EPIPE)
 		{
+		  AUTO_STRING (format, "[debug] EPIPE: %S");
+		  CALLN (Fmessage, format, p->name);
+
 		  p->raw_status_new = 0;
 		  pset_status (p, list2 (Qexit, make_fixnum (256)));
 		  p->tick = ++process_tick;
+		  read_process_output (proc, p->infd);
 		  deactivate_process (proc);
 		  error ("Process %s no longer connected to pipe; closed it",
 			 SDATA (p->name));
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 11efb428799..9cc6a837b30 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -744,15 +744,16 @@ eshell-output-object-to-target
   (condition-case err
       (process-send-string target object)
     (error
-     ;; If `process-send-string' raises an error and the process has
-     ;; finished, treat it as a broken pipe.  Otherwise, just re-raise
-     ;; the signal.  NOTE: When running Emacs in batch mode
-     ;; (e.g. during regression tests), Emacs can abort due to SIGPIPE
-     ;; here.  Maybe `process-send-string' should handle SIGPIPE even
-     ;; in batch mode (bug#66186).
-     (if (process-live-p target)
-         (signal (car err) (cdr err))
-       (signal 'eshell-pipe-broken (list target)))))
+     ;; NOTE: When running Emacs in batch mode (e.g. during regression
+     ;; tests), Emacs can abort due to SIGPIPE here.  Maybe
+     ;; `process-send-string' should handle SIGPIPE even in batch mode
+     ;; (bug#66186).
+
+     ;; FIXME: This used to signal `eshell-pipe-broken' only when the
+     ;; process was dead, so that transient errors in
+     ;; `process-send-string' wouldn't close our Eshell pipe.  Are there
+     ;; even transient errors like that, or what I overthinking things?
+     (signal 'eshell-pipe-broken (list target))))
   object)
 
 (cl-defmethod eshell-output-object-to-target (object
diff --git a/src/process.c b/src/process.c
index e61ec425f7e..56d86b2592e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -6873,10 +6873,13 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len,
 		}
 	      else if (errno == EPIPE)
 		{
-		  p->raw_status_new = 0;
-		  pset_status (p, list2 (Qexit, make_fixnum (256)));
-		  p->tick = ++process_tick;
-		  deactivate_process (proc);
+		  /* FIXME: Handle PTY too?  What about sockets?  And
+		     remove this debug statement.  */
+		  AUTO_STRING (format, "[debug] EPIPE: %S");
+		  CALLN (Fmessage, format, p->name);
+
+		  close_process_fd(&p->open_fd[WRITE_TO_SUBPROCESS]);
+		  p->outfd = -1;
 		  error ("Process %s no longer connected to pipe; closed it",
 			 SDATA (p->name));
 		}