Subject: bug#79436: [PATCH] Use up-to-date time in wait_reading_process_output

From: Spencer Baugh <[email-masked]>
Date: Fri, 12 Sep 2025 11:38:29 -0400

Tags: patch

Compile the following C program, close_and_sit.c:

#include <stdio.h>
#include <unistd.h>
void main() {
  fclose(stdout);
  fclose(stderr);
  while (1) sleep(1);
}

Then run this snippet of Lisp, updating the executable file name
appropriately:

(make-process
 :name "close_and_sit"
 :command '("./close_and_sit")
 :connection-type 'pipe)
(make-process :name "true" :command '("true"))
(message "accept-process-output")
(accept-process-output nil 1)

This will cause Emacs to hang; accept-process-output will never time
out.

There are two bugs which combine to cause this.

The first bug is in our timeout handling.  The attached patch resolves
that, which causes accept-process-output to time out properly and fixes
the bug.

Paul, this patch partially reverts a change you made to cache
current_timespec.  Does it seem right to you?

The second bug is that when a child process closes stdout/stderr, we
continue watching the pipe, generating spurious events.  (We should
still be timing out despite the spurious events - real events could also
cause this hang, this bug just makes it easier to trigger the hang.)
I'm still working on a patch to fix the second bug.

>From 338810c5ac4f9831bf1cff23a965ac0d460f9f7f Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Fri, 12 Sep 2025 11:18:28 -0400
Subject: [PATCH] Use up-to-date time in wait_reading_process_output

In ad23626030f0541e761f683661b51152128fb0b5 we changed to call
current_timespec less frequently, and instead cached it in
"now".  However, this is buggy: we only updated now when timeout
was non-zero, but timeout can be set temporarily to zero in
several places in wait_reading_process_output (such as when
checking for process status changes), which would cause us to
never update "now" and therefore never detect that a timeout
happened.

Also, this caching is wrong even in principle: since we call
Lisp code from wait_reading_process_output, substantial amounts
of time can pass, and we'd be left using an outdated "now" and
incorrectly not time out.  Also, the caching isn't a useful
performance optimization: current_timespec is fast.  So just get
rid of it.

* src/process.c (wait_reading_process_output): Stop caching
current_timespec.
---
 src/process.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/process.c b/src/process.c
index db7034fa98f..00658adcb28 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5347,9 +5347,6 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif
   specpdl_ref count = SPECPDL_INDEX ();
 
-  /* Close to the current time if known, an invalid timespec otherwise.  */
-  struct timespec now = invalid_timespec ();
-
   eassert (wait_proc == NULL
 	   || NILP (wait_proc->thread)
 	   || XTHREAD (wait_proc->thread) == current_thread);
@@ -5374,8 +5371,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   else if (time_limit > 0 || nsecs > 0)
     {
       wait = TIMEOUT;
-      now = current_timespec ();
-      end_time = timespec_add (now, make_timespec (time_limit, nsecs));
+      end_time = timespec_add (current_timespec (), make_timespec (time_limit, nsecs));
     }
   else
     wait = FOREVER;
@@ -5461,8 +5457,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       /* Exit if already run out.  */
       if (wait == TIMEOUT)
 	{
-	  if (!timespec_valid_p (now))
-	    now = current_timespec ();
+	  struct timespec now = current_timespec ();
 	  if (timespec_cmp (end_time, now) <= 0)
 	    break;
 	  timeout = timespec_sub (end_time, now);
@@ -5718,9 +5713,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	      && timespec_valid_p (timer_delay)
 	      && timespec_cmp (timer_delay, timeout) < 0)
 	    {
-	      if (!timespec_valid_p (now))
-		now = current_timespec ();
-	      struct timespec timeout_abs = timespec_add (now, timeout);
+	      struct timespec timeout_abs = timespec_add (current_timespec (), timeout);
 	      if (!timespec_valid_p (got_output_end_time)
 		  || timespec_cmp (timeout_abs, got_output_end_time) < 0)
 		got_output_end_time = timeout_abs;
@@ -5729,10 +5722,6 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	  else
 	    got_output_end_time = invalid_timespec ();
 
-	  /* NOW can become inaccurate if time can pass during pselect.  */
-	  if (timeout.tv_sec > 0 || timeout.tv_nsec > 0)
-	    now = invalid_timespec ();
-
 #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS
 	  if (retry_for_async
 	      && (timeout.tv_sec > 0 || timeout.tv_nsec > ASYNC_RETRY_NSEC))
@@ -5902,8 +5891,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	    }
 	  if (timespec_cmp (cmp_time, huge_timespec) < 0)
 	    {
-	      now = current_timespec ();
-	      if (timespec_cmp (cmp_time, now) <= 0)
+	      if (timespec_cmp (cmp_time, current_timespec ()) <= 0)
 		break;
 	    }
 	}
-- 
2.43.7