Subject: Re: bug#79201: 30.1.90; set-process-thread can permanently break fd_callback_info slots

From: Spencer Baugh <[email-masked]>
Date: Thu, 14 Aug 2025 12:21:15 -0400

Spencer Baugh <[email-masked]> writes:
> One theory: clear_waiting_thread_info only clears .waiting_thread up to
> max_desc.

OK, I figured it out.  This is indeed the sole issue.  With this patch:

diff --git a/src/process.c b/src/process.c
index 1a5c99139a6..5db427a4a99 100644
--- a/src/process.c
+++ b/src/process.c
@@ -670,10 +670,15 @@ clear_waiting_thread_info (void)
   int fd;
 
   eassert (max_desc < FD_SETSIZE);
-  for (fd = 0; fd <= max_desc; ++fd)
+  for (fd = 0; fd < FD_SETSIZE; ++fd)
     {
-      if (fd_callback_info[fd].waiting_thread == current_thread)
+      if (fd_callback_info[fd].waiting_thread == current_thread) {
+	if (fd > max_desc) {
+	  fprintf (stderr, "fd too high: %d > max_desc=%d\n", fd, max_desc);
+	  terminate_due_to_signal (SIGABRT, INT_MAX);
+	}
 	fd_callback_info[fd].waiting_thread = NULL;
+      }
     }
 }
 
And my earlier reproducer (but not my earlier patch), we quickly get an
abort, indicating that there's an fd_callback_info slot which would be
left with a non-NULL .waiting_thread.

The cause is that recompute_max_desc runs when a file descriptor is
deleted, and reduces max_desc.  But when it does so, that means
clear_waiting_thread_info will not get the chance to clear the
waiting_thread field in fd_callback_info slots above the new max_desc.

My initial thought was that recompute_max_desc should zero the
fd_callback_info[fd] entry when it reduces max_desc.  But it turns out
to be equivalent, and simpler, to zero fd_callback_info[desc] completely
instead of just setting flags to 0 right before we call
recompute_max_desc.

I do this in the attached patch, which fixes the bug.

>From 975741cc141c45c4d857f51098a35d98885920f4 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Thu, 14 Aug 2025 12:17:23 -0400
Subject: [PATCH] Zero fd_callback_info when deleting an fd

.waiting_thread and .thread could be left set to non-NULL values
in a deleted fd_callback_info entry.  These would never be
cleared by e.g. clear_waiting_thread_info since that only clears
fd_callback_info entries up to max_desc.  Clear fd_callback_info
entirely when deleting an entry.

* src/process.c (delete_write_fd)
(delete_keyboard_wait_descriptor): Set fd_callback_info
completely to zero.  (bug#79201)
(deactivate_process): Remove unnecessary recompute_max_desc.
---
 src/process.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/process.c b/src/process.c
index 1a5c99139a6..36717e39289 100644
--- a/src/process.c
+++ b/src/process.c
@@ -574,8 +574,7 @@ delete_write_fd (int fd)
   fd_callback_info[fd].flags &= ~(FOR_WRITE | NON_BLOCKING_CONNECT_FD);
   if (fd_callback_info[fd].flags == 0)
     {
-      fd_callback_info[fd].func = 0;
-      fd_callback_info[fd].data = 0;
+      fd_callback_info[fd] = (struct fd_callback_data) {};
 
       if (fd == max_desc)
 	recompute_max_desc ();
@@ -4809,8 +4808,6 @@ deactivate_process (Lisp_Object proc)
       delete_read_fd (inchannel);
       if ((fd_callback_info[inchannel].flags & NON_BLOCKING_CONNECT_FD) != 0)
 	delete_write_fd (inchannel);
-      if (inchannel == max_desc)
-	recompute_max_desc ();
     }
 }
 
@@ -8282,7 +8279,7 @@ delete_keyboard_wait_descriptor (int desc)
 #ifdef subprocesses
   eassert (desc >= 0 && desc < FD_SETSIZE);
 
-  fd_callback_info[desc].flags &= ~(FOR_READ | KEYBOARD_FD | PROCESS_FD);
+  fd_callback_info[desc] = (struct fd_callback_data) {};
 
   if (desc == max_desc)
     recompute_max_desc ();
-- 
2.39.3