GNU bug report logs - #79367
31.0.50; magit-commit sometimes doesn't work if diff-hl-update-async is t

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: i <at> fuzy.me, 79367 <at> debbugs.gnu.org, dmitry <at> gutov.dev
Subject: Re: bug#79367: 31.0.50; magit-commit sometimes doesn't work if
 diff-hl-update-async is t
Date: Wed, 03 Sep 2025 19:42:47 +0300

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: i <at> fuzy.me,  79367 <at> debbugs.gnu.org,  dmitry <at> gutov.dev
> Date: Wed, 03 Sep 2025 12:03:42 -0400
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> >> But I think we need to call set_proc_thread when the server process
> >> >> is not locked as well.  Otherwise the `thread' member of the
> >> >> fd_callback_info may still be a dangling pointer.
> >> >
> >> > Oh, you mean the below?  I agree, it is safer.
> >> 
> >> We should not do that - if the thread member of fd_callback_info is a
> >> dangling pointer, that indicates there's a bug elsewhere.
> >
> > So how about adding an assertion before we clear it?
> 
> Sure.  fd_callback_info.thread specifically should always be NULL at the
> time we're using the fd_callback_info slot for some new fd, so we should
> assert that.

So, this:

diff --git a/src/process.c b/src/process.c
index d6efac5..8f5be5e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5078,6 +5078,10 @@ server_accept_connection (Lisp_Object server, int channel)
   fcntl (s, F_SETFL, O_NONBLOCK);
 
   p = XPROCESS (proc);
+  /* make_process calls pset_thread, but if the server process is not
+     locked to any thread, we need to undo what make_process did.  */
+  if (NILP (ps->thread))
+    pset_thread (p, Qnil);
 
   /* Build new contact information for this setup.  */
   contact = Fcopy_sequence (ps->childp);
@@ -5117,6 +5121,19 @@ server_accept_connection (Lisp_Object server, int channel)
     add_process_read_fd (s);
   if (s > max_desc)
     max_desc = s;
+  /* If the server process is locked to this thread, lock the client
+     process to the same thread, otherwise clear the thread of its I/O
+     descriptors.  */
+  if (NILP (ps->thread))
+    {
+      eassert (!fd_callback_info[p->infd].thread);
+      set_proc_thread (p, NULL);
+    }
+  else
+    {
+      eassert (XTHREAD (ps->thread) == current_thread);
+      set_proc_thread (p, XTHREAD (ps->thread));
+    }
 
   /* Setup coding system for new process based on server process.
      This seems to be the proper thing to do, as the coding system

Previous Next

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