Subject: Re: bug#79334: [PATCH] Don't release thread select lock unnecessarily

From: Spencer Baugh <[email-masked]>
Date: Fri, 29 Aug 2025 11:43:21 -0400

On Fri, Aug 29, 2025, 11:41 AM Eli Zaretskii <[email-masked]> wrote:

> > From: Spencer Baugh <[email-masked]>
> > Cc: [email-masked],  [email-masked],  [email-masked]
> > Date: Fri, 29 Aug 2025 10:49:34 -0400
> >
> > Eli Zaretskii <[email-masked]> writes:
> >
> > >> Primarily: Calling thread_select here complicates reasoning about the
> > >> code.  We should not release the lock when not necessary, because it
> > >> makes it harder to think about possible thread interleavings.  And
> it's
> > >> not necessary here because this pselect call returns immediately.
> > >
> > > How do we define "necessary" in this context?
> >
> > It's necessary to release the lock around long pselect calls because
> > otherwise other threads will not run while Emacs is waiting for input,
> > which means they probably won't run at all.
> >
> > >> (The only reason the pselect call wouldn't return immediately is if we
> > >> released the lock and then switched to another thread)
> > >
> > > You seem to assume that a call to thread_select is only necessary when
> > > some potentially prolonged operation or wait is expected?  But I'm not
> > > sure this is correct, because that could prevent other threads from
> > > running for too long, and force programmers to inject sleep-for
> > > etc. in their programs.  By allowing thread switches as frequently as
> > > possible lets other threads more opportunities to run (to some extent
> > > simulating preemptive scheduling), which I think is a Good Thing
> > > overall, no?
> >
> > Three points:
> >
> > - Most importantly: We can only allow thread switches at known safe
> >   points.  It's not clear that this is a safe point.  (Actually, we know
> >   it currently isn't, because of the status_notify issue.  But I suspect
> >   there may be other issues which are harder to analyze)
>
> This reason will be removed once we fix status_notify.
>
> > - Adding more thread switches is not necessarily a good thing.  It can
> >   degrade whole-system performance, because context switching is slow.
> >
> > - wait_reading_process_output already does a thread switch around the
> >   pselect, so it's not important for one to happen here.
>
> You basically give the same single reason in several different
> wordings.
>
> I see your point, but my experience with threads in Emacs is the
> opposite: there seem to be too few thread switch opportunities, so
> many programs don't work as expected unless one sprinkles sleep-for in
> various places.  For example, try to run something in a non-main
> thread and then attempt to interact with Emacs in the main thread.
>
> IOW, it is quite easy for a Lisp program to hog the global lock and
> starve the other threads, so each additional thread-switch opportunity
> is IME a blessing, not a curse.
>

Okay, so then how about we delete this unsafe thread_select and put some
thread-yield calls around wait_reading_process_output?