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

View this message in rfc822 format

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: bug#79367: 31.0.50; magit-commit sometimes doesn't work if diff-hl-update-async is t
Date: Tue, 02 Sep 2025 18:02:39 +0300

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: i <at> fuzy.me,  dmitry <at> gutov.dev,  79367 <at> debbugs.gnu.org
> Date: Tue, 02 Sep 2025 10:33:12 -0400
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > 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.
> 
> True, that's a bug.
> 
> > 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.
> 
> Not if the existing process was unlocked.  Then this is a random thread.

Then let's fix the case of an unlocked process (which AFAIU is not
what happened here).  But if the process was locked to a particular
thread, as that is the default, then do you agree that this code
should be run by that very thread, at least nominally?

> >> 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...
> 
> No, we're not past that, I still think those changes are wrong.
> Especially because this bug report shows that those changes are indeed
> breaking existing code.
> 
> I repeatedly said that those changes would cause new issues to be
> reported, and now it is happening.  Maybe it doesn't convince you that
> the changes need to be reverted, but you should at least be giving it a
> little more credence now.

This particular breakage is simply because my change was incomplete:
it left some processes not locked to their thread, and the solution
proposed by the OP, which fixed that blunder, is simpler and has fewer
downsides than backing out the process locking to threads.  This
happens all the time in development, and the conclusion is rarely that
the original changes should be backed out, certainly not if there are
simpler fixes.

Previous Next

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