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 #20 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, dmitry <at> gutov.dev, 79367 <at> debbugs.gnu.org
Subject: Re: bug#79367: 31.0.50; magit-commit sometimes doesn't work if
 diff-hl-update-async is t
Date: Tue, 02 Sep 2025 17:17:38 +0300

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: Zhengyi Fu <i <at> fuzy.me>,  Dmitry Gutov <dmitry <at> gutov.dev>,
>    79367 <at> debbugs.gnu.org
> Date: Tue, 02 Sep 2025 09:08:14 -0400
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> --- a/src/process.c
> >> +++ b/src/process.c
> >> @@ -5115,6 +5115,8 @@ server_accept_connection (Lisp_Object server, int channel)
> >>    /* Client processes for accepted connections are not stopped initially.  */
> >>    if (!EQ (p->filter, Qt))
> >>      add_process_read_fd (s);
> >> +  set_proc_thread (p, current_thread);
> >> +  
> >>    if (s > max_desc)
> >>      max_desc = s;
> >
> > Thanks.
> >
> > I believe you are right, since server_accept_connection calls
> > make_process to create a new process object, which should by default
> > be locked to the thread which created it.
> >
> > Spencer and Dmitry, do you agree?
> 
> No.  At worst, the locking of the new connection should match that of
> the existing process.  The new connection should not randomly be locked
> to the thread which happened to run wait_reading_process_output.

But make_process, called by server_accept_connection, already did
this:

  pset_thread (p, Fcurrent_thread ());

So if we want to lock this new process to some other thread, we should
fix that part as well to be consistent.

And why do you think this thread is a random one?  If everything works
as intended, it is the thread which created the existing process,
because the descriptor whose presence in Available triggers the call
to server_accept_connection is one of those on which pselect waited,
and should have belonged to the current thread.  If that is not true,
then it's a separate problem which should be fixed, perhaps as part of
the related discussions.

> >> Further debugging through gdb revealed more details: the `thread'
> >> member of the fd_callback_info entry for the server connection
> >> references the diff-hl-async thread, which is already exited.
> 
> This aspect sounds similar to bug#79201.
> 
> Zhengyi, I assume you're testing with an Emacs built from source.  Can
> you check whether the Emacs you're building includes commit
> 37325ed5a9c7f62c35b368d9530496ed31404940, which was pushed just a couple
> weeks ago?  If not, can you pull and test again?
> 
> If that doesn't fix it (or you already had that commit), can you try
> reverting commits 034d755f2f21088b97fdb0a34d846c39fcdbf46d then
> c93be71e45d4cebeb017c813426228e579e9381d and test again?

What will that prove?  Those changes are not going away, so trying
that is just wasting everyone's time and energy.  I thought we were
past that...

Previous Next

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