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

From: Dmitry Gutov <[email-masked]>
Date: Tue, 2 Sep 2025 22:45:54 +0300

On 02/09/2025 22:18, Eli Zaretskii wrote:

>> I do believe that when a change in a long-standing implementation causes
>> an immediate issue we first consider reverting, especially when there is
>> no consensus on the approach.
> 
> No, we don't.  Reverting is actually the last resort, reserved for
> plain and clear mistakes, and for changes that cause grave regressions
> and cannot be fixed soon enough.

A "mistake" is often in the eyes of the beholder. The lack of consensus 
is something more objective.

>> A one-liner change is probably not a big deal by itself, but like
>> Spencer said, the new process should at least follow the locking of its
>> original thread, or something like this. And we still don't have a
>> proposed fix on that.
> 
> I can code the proposed fix in a few minutes, if there's agreement to
> what I proposed to do.  I'm still uncertain what I proposed is
> agreed-upon.  So let me reiterate that:
> 
>    . if the process on behalf of which we called
>      server_accept_connection was not locked to some thread, undo what
>      make_process did to the new process when it called pset_thread
>    . otherwise, call set_proc_thread to lock the new process to the
>      same thread as the one which caused this call

Sounds about right.

>> I don't want us to spend a lot of time arguing revert-or-no-revert now,
>> but could you, Eli, try to decide on a full example of a buggy scenario
>> which is really solved by thread locking being?
> 
> I already described that in so many words.  That is all I can afford.

We need a fuller scenario. If not with code, then a full use case 
described in English.

I'm pretty sure more of your time is being wasted on answering the 
emails with objections. Regrettably.

> And please keep in mind that processes were being locked to threads
> since day one, just incompletely so.  I didn't invent it just
> yesterday out of the blue: it's the original design of processes with
> threads.

IIUC the existing protection did very little in practice, all of these 
years: it only stopped some thread calling 'accept-process-output' with 
an explicit reference of a process belonging to another thread. Any kind 
of buggy code would have to try hard to obtain that reference in the 
first place - so that would almost never happen anyway. The new 
protections seem a lot more strict.

TBC I think we should continue to support locking, and your current fix 
seems productive in this regard, but whether locking should be by 
default is not obvious to me - and at least two smart devs who worked 
with Emacs Lisp threads say no. That seems like grounds to re-evaluate. 
Even if we reach the current default we'll have documented our 
conclusions better rather than just saying "this is so".