GNU bug report logs - #79201
30.1.90; set-process-thread can permanently break fd_callback_info slots

Previous Next

Full log

Message #11 received at 79201 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: "John Wiegley" <johnw <at> gnu.org>
Cc: 79201 <at> debbugs.gnu.org, dmitry <at> gutov.dev, app-emacs-dev <at> janestreet.com
Subject: Re: bug#79201: 30.1.90; set-process-thread can permanently break
 fd_callback_info slots
Date: Fri, 08 Aug 2025 16:34:10 -0400

[Message part 1 (text/plain, inline)]

"John Wiegley" <johnw <at> gnu.org> writes:

>>>>>> Spencer Baugh <sbaugh <at> janestreet.com> writes:
>
>> (while (null (accept-process-output proc2)))
>
> Based on the what the manual says, shouldn’t this be?
>
>     (while (accept-process-output proc2))

No, I'm trying to run accept-process-output repeatedly until proc2
produces any output at all, which will cause accept-process-output to
return non-nil.

>> Anyway, probably we should be setting fd_callback_info[fd].thread to
>> NULL also when the process exits.  But, I suggest we should further also
>> set it to NULL (and also waiting_thread) when we start using a
>> fd_callback_info[fd] slot, e.g. in add_read_fd.  That avoids the
>> possibility of permanent contamination of fd_callback_info slots, which
>> I think is possible in some other ways, though I haven't been able to
>> reproduce it yet...
>
> This sounds like an important fix to me. I haven’t run into this problem yet
> myself, but the logic above tracks.

Here's a patch to fix it by initializing these fields.

[0001-Initialize-fd_callback_info-thread-variables-on-reus.patch (text/x-patch, inline)]

From d97986f4a46a229c3a19a4e554e16b2104018388 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Fri, 8 Aug 2025 16:33:20 -0400
Subject: [PATCH] Initialize fd_callback_info thread variables on reuse

By using set-process-thread and then delete-process,
fd_callback_info[fd].thread will contain a dangling pointer to a
dead thread, making that fd number permanently broken and Emacs
unable to read output from it.

Handle all such issues by initializing .thread and
.waiting_thread to NULL at the time we start using an
fd_callback_info[fd] slot.

* src/process.c (initialize_fd_callback_info): Add.
(add_read_fd, add_process_read_fd, add_write_fd)
(add_non_blocking_write_fd, add_keyboard_wait_descriptor): Call
initialize_fd_callback_info. (bug#79201)
---
 src/process.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/process.c b/src/process.c
index e61ec425f7e..d5d0db1943e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -467,6 +467,15 @@ make_lisp_proc (struct Lisp_Process *p)
 } fd_callback_info[FD_SETSIZE];
 
 
+static void
+initialize_fd_callback_info(int fd)
+{
+  eassert (0 <= fd && fd < FD_SETSIZE);
+
+  fd_callback_info[fd].thread = NULL;
+  fd_callback_info[fd].waiting_thread = NULL;
+}
+
 /* Add a file descriptor FD to be monitored for when read is possible.
    When read is possible, call FUNC with argument DATA.  */
 
@@ -475,7 +484,6 @@ add_read_fd (int fd, fd_callback func, void *data)
 {
   add_keyboard_wait_descriptor (fd);
 
-  eassert (0 <= fd && fd < FD_SETSIZE);
   fd_callback_info[fd].func = func;
   fd_callback_info[fd].data = data;
 }
@@ -490,14 +498,13 @@ add_non_keyboard_read_fd (int fd, fd_callback func, void *data)
 static void
 add_process_read_fd (int fd)
 {
-  eassert (fd >= 0 && fd < FD_SETSIZE);
+  initialize_fd_callback_info(fd);
   eassert (fd_callback_info[fd].func == NULL);
 
   fd_callback_info[fd].flags &= ~KEYBOARD_FD;
   fd_callback_info[fd].flags |= FOR_READ;
   if (fd > max_desc)
     max_desc = fd;
-  eassert (0 <= fd && fd < FD_SETSIZE);
   fd_callback_info[fd].flags |= PROCESS_FD;
 }
 
@@ -522,7 +529,7 @@ delete_read_fd (int fd)
 void
 add_write_fd (int fd, fd_callback func, void *data)
 {
-  eassert (fd >= 0 && fd < FD_SETSIZE);
+  initialize_fd_callback_info(fd);
 
   fd_callback_info[fd].func = func;
   fd_callback_info[fd].data = data;
@@ -534,7 +541,7 @@ add_write_fd (int fd, fd_callback func, void *data)
 static void
 add_non_blocking_write_fd (int fd)
 {
-  eassert (fd >= 0 && fd < FD_SETSIZE);
+  initialize_fd_callback_info(fd);
   eassert (fd_callback_info[fd].func == NULL);
 
   fd_callback_info[fd].flags |= FOR_WRITE | NON_BLOCKING_CONNECT_FD;
@@ -8296,7 +8303,7 @@ remove_slash_colon (Lisp_Object name)
 add_keyboard_wait_descriptor (int desc)
 {
 #ifdef subprocesses /* Actually means "not MSDOS".  */
-  eassert (desc >= 0 && desc < FD_SETSIZE);
+  initialize_fd_callback_info(desc);
   fd_callback_info[desc].flags &= ~PROCESS_FD;
   fd_callback_info[desc].flags |= (FOR_READ | KEYBOARD_FD);
   if (desc > max_desc)
-- 
2.39.3

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.