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

From: Spencer Baugh <[email-masked]>
Date: Mon, 11 Aug 2025 13:58:02 -0400

Eli Zaretskii <[email-masked]> writes:

>> From: Spencer Baugh <[email-masked]>
>> Cc: [email-masked],  [email-masked],  [email-masked],
>>    [email-masked]
>> Date: Mon, 11 Aug 2025 13:20:05 -0400
>> 
>> Yes, sorry, I missed that there are two separate mechanisms.  I think
>> set-process-thread should continue setting the thread member of the
>> process object, but not the thread member of the fd_callback_info struct.
>
> Why not?  set-process-thread can be called when the descriptors for
> the process are already set up.  That's unlike make-process, which
> indeed doesn't set the .thread member of the descriptors.

It's useless and causes this bug.

>> Yes.  Let's just make set-process-thread stop setting the thread member
>> of the fd_callback_info struct.
>
> I don't agree it shouldn't see above.  What problems do you see as the
> result of that?

It causes this bug.

Here's an attached patch which makes this change.  The thread member of
fd_callback_info is then never set, so it can be entirely deleted.

This doesn't need an announcement, because the behavior of "calling
accept-process-output on a process locked to a different thread" still
works the same way it always did.  There's no (non-bug) user-visible
change.

>From b5311d3af0d020f718b62f239f9e360379276230 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Mon, 11 Aug 2025 13:54:14 -0400
Subject: [PATCH] Remove thread member of fd_callback_info

Previously, if set-process-thread was called on a process after its
creation, we would set the thread field in the fd_callback_info for that
process's file descriptors.

This behavior isn't useful, and cauases bugs since this field could stay
set to a dangling pointer if delete-process is called before thread
exit.  So simply remove it.

* src/process.c (compute_input_wait_mask)
(compute_non_process_wait_mask, compute_non_keyboard_wait_mask)
(compute_write_mask, update_processes_for_thread_death)
(Fset_process_thread): Remove thread member of fd_callback_info
(bug#79201)
---
 src/process.c | 34 ++--------------------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/src/process.c b/src/process.c
index e61ec425f7e..1a5c99139a6 100644
--- a/src/process.c
+++ b/src/process.c
@@ -591,9 +591,6 @@ compute_input_wait_mask (fd_set *mask)
   eassert (max_desc < FD_SETSIZE);
   for (fd = 0; fd <= max_desc; ++fd)
     {
-      if (fd_callback_info[fd].thread != NULL
-	  && fd_callback_info[fd].thread != current_thread)
-	continue;
       if (fd_callback_info[fd].waiting_thread != NULL
 	  && fd_callback_info[fd].waiting_thread != current_thread)
 	continue;
@@ -614,9 +611,6 @@ compute_non_process_wait_mask (fd_set *mask)
   eassert (max_desc < FD_SETSIZE);
   for (fd = 0; fd <= max_desc; ++fd)
     {
-      if (fd_callback_info[fd].thread != NULL
-	  && fd_callback_info[fd].thread != current_thread)
-	continue;
       if (fd_callback_info[fd].waiting_thread != NULL
 	  && fd_callback_info[fd].waiting_thread != current_thread)
 	continue;
@@ -638,9 +632,6 @@ compute_non_keyboard_wait_mask (fd_set *mask)
   eassert (max_desc < FD_SETSIZE);
   for (fd = 0; fd <= max_desc; ++fd)
     {
-      if (fd_callback_info[fd].thread != NULL
-	  && fd_callback_info[fd].thread != current_thread)
-	continue;
       if (fd_callback_info[fd].waiting_thread != NULL
 	  && fd_callback_info[fd].waiting_thread != current_thread)
 	continue;
@@ -662,9 +653,6 @@ compute_write_mask (fd_set *mask)
   eassert (max_desc < FD_SETSIZE);
   for (fd = 0; fd <= max_desc; ++fd)
     {
-      if (fd_callback_info[fd].thread != NULL
-	  && fd_callback_info[fd].thread != current_thread)
-	continue;
       if (fd_callback_info[fd].waiting_thread != NULL
 	  && fd_callback_info[fd].waiting_thread != current_thread)
 	continue;
@@ -976,12 +964,6 @@ update_processes_for_thread_death (Lisp_Object dying_thread)
 	  struct Lisp_Process *proc = XPROCESS (process);
 
 	  pset_thread (proc, Qnil);
-	  eassert (proc->infd < FD_SETSIZE);
-	  if (proc->infd >= 0)
-	    fd_callback_info[proc->infd].thread = NULL;
-	  eassert (proc->outfd < FD_SETSIZE);
-	  if (proc->outfd >= 0)
-	    fd_callback_info[proc->outfd].thread = NULL;
 	}
     }
 }
@@ -1451,25 +1433,13 @@ DEFUN ("set-process-thread", Fset_process_thread, Sset_process_thread,
   (Lisp_Object process, Lisp_Object thread)
 {
   struct Lisp_Process *proc;
-  struct thread_state *tstate;
 
   CHECK_PROCESS (process);
-  if (NILP (thread))
-    tstate = NULL;
-  else
-    {
-      CHECK_THREAD (thread);
-      tstate = XTHREAD (thread);
-    }
+  if (!NILP (thread))
+    CHECK_THREAD (thread);
 
   proc = XPROCESS (process);
   pset_thread (proc, thread);
-  eassert (proc->infd < FD_SETSIZE);
-  if (proc->infd >= 0)
-    fd_callback_info[proc->infd].thread = tstate;
-  eassert (proc->outfd < FD_SETSIZE);
-  if (proc->outfd >= 0)
-    fd_callback_info[proc->outfd].thread = tstate;
 
   return thread;
 }
-- 
2.39.3