all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Spencer Baugh <sbaugh@janestreet.com>
To: emacs-devel@gnu.org
Subject: Re: call-process should not block process filters from running
Date: Sat, 01 Jul 2023 14:24:59 -0400	[thread overview]
Message-ID: <ierv8f3pk04.fsf@janestreet.com> (raw)
In-Reply-To: CAO=BR8NaV3oMCyL9WJu9yA+jFoJXf3ePXwmuvvHUd-ocy9MVZA@mail.gmail.com

Spencer Baugh <sbaugh@janestreet.com> writes:
> On Wed, Jun 28, 2023 at 8:52 AM Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> > From: Spencer Baugh <sbaugh@janestreet.com>
>> > Cc: app-emacs-dev@janestreet.com
>> > Date: Tue, 27 Jun 2023 17:55:00 -0400
>> >
>> >
>> > When Lisp code calls call-process, then while call-process is running,
>> > all Lisp is blocked from running, including process filters and timers
>> > created beforehand by other Lisp.  This call-process behavior is
>> > harmful, but we can fix call-process to not behave this way.
>> >
>> > This call-process behavior is harmful:
>> > Many packages rely on their process filters and timers being able to
>> > run:
>> > - Packages which communicate over the network (such as ERC)
>> >   rely on being able to respond to heartbeats and prevent timeouts.
>> > - Packages which respond to requests from other programs (such as EXWM)
>> >   rely on being able to respond to those requests.
>> > A simple (shell-command "sleep 60") will cause most such packages to
>> > break or behave poorly.
>>
>> Packages that rely on the above should be defensive in the face of
>> delays, because Emacs calls blocking APIs all over the place;
>> call-process is just one of them.  Moreover, a long-running Lisp
>> program will also block timers and async communications, because Emacs
>> can only be relied upon to run timers and read subprocess output when
>> it is idle; otherwise we need some special calls or signals to arrive
>> to trigger reading input that waits.
>>
>> Therefore, I think the above exaggerates the problem, and also
>> "blames" a single API for something that is ubiquitous in Emacs Lisp
>> programming and should be accounted for by any package that is
>> sensitive to delays.
>>
>> > My suggestion is that we should create a new helper function in Lisp,
>> > perhaps called "process-run", which has an identical interface as
>> > call-process, except that while it is running, other process filters and
>> > other Lisp are still able to run.  Then we can move users of
>> > call-process over to this new function.  Most (but perhaps not all) Lisp
>> > using call-process should be using process-run, since most Lisp doesn't
>> > actually want to block process filters from running.
>>
>> I have no objections to adding new functionality that could make
>> call-process less blocking (but rewriting it in Lisp based on
>> make-process and accept-process-output is not the only possible
>> implementation, see below).  But I am firmly against any massive
>> moving of the current users of call-process to this new functionality,
>> for several reasons:
>>
>>   . some Lisp programs might rely on the fact that no other Lisp runs
>>     while the sub-process is running, and no input from any source but
>>     that subprocess can arrive;
>>   . using async subprocesses has its downsides:
>>     - it is hard to read only from a given subprocess without causing
>>       issues to other filters
>>     - reading from async subprocess has known problems when you need
>>       to decode its output
>>     - the timing and synchronization is problematic when using
>>       accept-process-output, and has its pitfalls
>>     - on Windows the number of simultaneous async subprocesses is
>>       limited by a relatively small number
>>   . reimplementing this in Lisp will increase consing, which will
>>     trigger more GCs, which will slow down callers of this new API wrt
>>     call-process
>>
>> So I think we must consider each caller of call-process separately, on
>> a case by case basis, and only switch where (a) the process can indeed
>> take a long time, and (b) after careful auditing of the code and its
>> expectations from the subprocess call.
>>
>> > - This does not require changing the C core of Emacs at all, nor
>> >   changing the call-process implementation.  The existing
>> >   accept-process-output API is sufficient for creating a synchronous
>> >   process-running API which does not block Lisp from running in process
>> >   filters and timers and so on.
>>
>> I would actually suggest to consider a different approach, which does
>> need changes in C (but they are relatively simple changes).
>> Reimplementing all the complexities of the C code (don't forget
>> call-process-region) in Lisp will definitely cause bugs and
>> destabilize features that worked flawlessly for decades, so if a way
>> exists that allows us to have a "less blocking" call-process, without
>> requiring such massive reimplementation, I think we should seriously
>> consider it.
>>
>> And AFAIU, such a way does exist.  The implementation of call-process
>> actually forks a subprocess, then reads from its pipe until EOF, and
>> then waits in waitpid for the subprocess to exit.  Both the reading
>> and the waiting are done in a loop.  So one way of making call-process
>> less blocking is by adding to these loops calls to
>> wait_reading_process_output, like we do, for example, in sleep-for,
>> but conditioned by some new variable exposed to Lisp.  Lisp programs
>> which want this behavior could then activate it by binding that new
>> variable.  This could give us most or all of the advantages of
>> non-blocking API without most disadvantages.  (We'd still need to move
>> to this functionality on a case by case basis, because some of the
>> considerations against that could still be valid in individual cases.)
>
> This sounds great, I would be happy to implement this.  I think we
> would also want a tiny wrapper in Lisp which binds this new variable
> then calls call-process, rather than having lots of programs binding
> the variable directly, to make it easier to change the implementation
> strategy in the future.
>
> I'll work on implementing this new variable.  If you have any other
> suggestions for it, let me know.

Just to sanity check before I go down the wrong path: When this variable
is set, instead of doing the reading from the subprocess's pipe in
call-process, we'll need to do it in wait_reading_process_output, so
that other Lisp can run.  We can't just add in calls to
wait_reading_process_output alongside the existing calls to read,
because read is blocking, and we need to process other input if there is
some, even if there's no input from the process under call_process.  A
similar change will need to happen for waitpid handling.




  parent reply	other threads:[~2023-07-01 18:24 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 21:55 call-process should not block process filters from running Spencer Baugh
2023-06-28 11:39 ` Mattias Engdegård
2023-06-28 11:56 ` Po Lu
2023-06-28 12:08   ` Spencer Baugh
2023-06-28 13:17     ` Po Lu
2023-06-28 12:52 ` Eli Zaretskii
2023-06-28 13:27   ` Spencer Baugh
2023-06-28 13:34     ` Eli Zaretskii
2023-07-01 18:24     ` Spencer Baugh [this message]
2023-07-01 18:59       ` Eli Zaretskii
2023-07-01 19:17         ` Spencer Baugh
2023-07-02  5:45           ` Eli Zaretskii
2023-07-03  0:02             ` sbaugh
2023-07-03 10:00               ` Po Lu
2023-07-03 17:53                 ` sbaugh
2023-07-03 18:51                   ` Eli Zaretskii
2023-07-03 20:28                     ` sbaugh
2023-07-04  4:12                       ` Po Lu
2023-07-04 11:25                         ` Eli Zaretskii
2023-07-04 12:42                         ` sbaugh
2023-07-04 13:42                           ` Michael Albinus
2023-07-04 14:16                             ` sbaugh
2023-07-05  6:36                               ` Michael Albinus
2023-07-04 11:10                       ` Eli Zaretskii
2023-07-04 12:20                         ` sbaugh
2023-07-04 13:09                           ` Eli Zaretskii
2023-07-04 13:37                             ` sbaugh
2023-07-04 13:25                           ` Po Lu
2023-07-04  1:04                     ` sbaugh
2023-07-04  4:09                       ` Po Lu
2023-07-04 12:27                         ` sbaugh
2023-07-04 13:22                           ` Po Lu
2023-07-04 13:51                             ` sbaugh
2023-07-04 16:38                               ` Eli Zaretskii
2023-07-04 16:53                                 ` sbaugh
2023-07-04 17:14                                   ` Eli Zaretskii
2023-07-04 16:49               ` Dmitry Gutov
2023-07-04 18:12                 ` sbaugh
2023-07-05 18:53                   ` Dmitry Gutov
2023-07-06  2:24                     ` sbaugh
2023-07-06  8:06                       ` Michael Albinus
2023-07-08 15:54                         ` sbaugh
2023-07-09  9:04                           ` Michael Albinus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ierv8f3pk04.fsf@janestreet.com \
    --to=sbaugh@janestreet.com \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.