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

From: Jim Porter <[email-masked]>
Date: Thu, 24 Jul 2025 10:05:50 -0700

On 7/24/2025 6:52 AM, Paul Eggert wrote:
> On 2025-07-23 22:34, Eli Zaretskii wrote:
>>> Date: Wed, 23 Jul 2025 11:06:24 -0700
>>> From: Jim Porter<[email-masked]>
> 
>>> In process.c,
>>> when we write to a process, we handle EPIPE errors by calling
>>> 'deactivate_process'. However, that can lead to us dropping any data
>>> written*by* that process, since we don't call the process's filter
>>> function for any remaining output still in our internal buffer.
>>>
>>> I think in this case, we'd just want to let the rest of our code handle
>>> deactivating the process in the usual way. That helps fix this bug, plus
>>> I think it makes sense in general. If a process closes stdin, I believe
>>> we'd get the EPIPE error, but that process might want to continue
>>> working (though it does mean that you could only interact with that
>>> process via signaling it).
> 
> Yes, that makes sense.

Thanks for checking. Then I think the attached diff would be close to 
what we want, with a few caveats. (The original diff was just the 
smallest change that seemed to fix the problem so that Daniel could try 
it out.)

This diff changes things so that when we get an EPIPE, we close our end 
of the pipe (the 'WRITE_TO_SUBPROCESS' fd). I don't know if we need to 
do anything special for PTYs or sockets though.

On the Eshell side then, we previously detected EPIPE by checking 
whether the process had been deactivated. Since that doesn't happen 
anymore, I changed it so that we always treat an error from 
'process-send-string' as though the pipe broke. In practice, this just 
means that Eshell will think that all future calls to 
'process-send-string' would fail too (and so it will send a SIGPIPE 
signal to the sending process; "git" in the original test case). I'm not 
totally sure that's right though. Could there be a time when we get an 
error from 'process-send-string', but we can try calling it again later?

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));
 		}