Subject: bug#79367: 31.0.50; magit-commit sometimes doesn't work if diff-hl-update-async is t

From: Spencer Baugh <[email-masked]>
Date: Tue, 02 Sep 2025 09:08:14 -0400

Eli Zaretskii <[email-masked]> writes:

>> From: Zhengyi Fu <[email-masked]>
>> Date: Tue, 02 Sep 2025 14:18:58 +0800
>> 
>> Hi Emacs maintainers,
>> 
>> I have been suffering from this bug for a few weeks.
>> 
>> I spent a few days finding minimal steps to reproduce the bug:
>> 
>> - Install diff-hl and magit
>> - Set `diff-hl-update-async' to t and enable global-diff-hl-mode
>> - Enable server-mode
>> 
>> - Open a file in a git repo, make some changes and save it
>> - Run `magit-stage-buffer-file' to stage the changes
>> - Run `revert-buffer' in the file buffer, and make sure the diff-hl
>>   fringes are displayed
>> - Run `magit-commit-create'
>> 
>> At this moment, a COMMIT_EDITMSG buffer should appear.  But actually
>> sometimes no buffer appears.
>> 
>> If you run `list-processes', you can see that the git process and the
>> server connection are all there, as shown below.
>> 
>> git             542014  run     magit-process: emacs_test /dev/pts/1   Main         git --no-pager --literal-pathspecs -c core.preloadindex=true -c log.showSignature=false -c color.ui=false -c color.diff=false -c diff.noPrefix=false commit --
>> server          --      listen  --                        --           Main         (network server on /run/user/150425139/emacs/server)
>> server <2>      --      open    --                        --           Main         (network connection to t:/run/user/150425139/emacs/server)
>> 
>> 
>> The commit message buffer will pop up, if you eval
>>    (set-process-thread (get-process "server <2>") nil)
>> 
>> 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.
>> 
>> Simply adding a `set_proc_thread' in `server_accept_connections' like
>> the following seems to resolve the issue, but I'm not sure if this is
>> the correct fix.
>> 
>> diff --git a/src/process.c b/src/process.c
>> index d6efac5479d..326a36716d5 100644
>> --- 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.

>> 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?