call-process should not block process filters from running

call-process should not block process filters from running

[Spencer Baugh]

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.

We can fix call-process to not behave this way:
It is straightforward to reimplement call-process in Lisp, using
accept-process-output, so that process filters and timers and other Lisp
can run while waiting on call-process.  There's been at least one
attempt at doing so:
https://github.com/tromey/emacs/tree/rewrite-call-process

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.

As a bonus, process-run will be usable by Lisp code running in Lisp
threads created by make-thread, as will any functions which use
process-run, since accept-process-output supports threads.
(call-process does not)

A few clarifications in advance for things which I suspect might be
confusing:

- This is not about switching Lisp from running processes synchronously
  (call-process) to asynchronously (make-process).  process-run would
  still be a synchronous API.  It is much easier to fix this one bad
  aspect of the call-process API than to move lots of Lisp over to
  make-process.  Even if we moved all Lisp over to be run asynchronous
  processes, we would still want a fixed call-process for Lisp thread
  support.

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

- MS-DOS can't support an implementation of call-process which allows
  other Lisp to run.  On MS-DOS, call-process and process-run will be
  identical.  This is fine and not a problem.  Furthermore, even if it
  was a problem, MS-DOS should not hold us back from improving Emacs on
  other platforms.

Re: call-process should not block process filters from running

[Mattias Engdegård]

27 juni 2023 kl. 23.55 skrev Spencer Baugh <sbaugh@janestreet.com>:

> 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

I/O is faster for call-process partly because these things can be avoided, and there is less consing. The overall latency is also slightly lower.
(Not to say that there isn't plenty of room for improvement for either.)

But otherwise yes, make-process is often preferable for the reasons you mention and others.

Re: call-process should not block process filters from running

[Po Lu]

Spencer Baugh <sbaugh@janestreet.com> writes:

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

Have you considered that many programs may conversely rely on the fact
that call-process can not run Lisp through selection converters or
process filters?

Re: call-process should not block process filters from running

[Spencer Baugh]

Po Lu <luangruo@yahoo.com> writes:
> Spencer Baugh <sbaugh@janestreet.com> writes:
>
>> 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.
>
> Have you considered that many programs may conversely rely on the fact
> that call-process can not run Lisp through selection converters or
> process filters?

Yes.  Those programs can continue to use call-process.  Others which
don't rely on this fact can be moved to a different interface which can
run Lisp, to avoid this downside.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> 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.)

Re: call-process should not block process filters from running

[Po Lu]

Spencer Baugh <sbaugh@janestreet.com> writes:

> Yes.  Those programs can continue to use call-process.  Others which
> don't rely on this fact can be moved to a different interface which can
> run Lisp, to avoid this downside.

Every time we move a C interface to Emacs Lisp, we end up with bugs that
destabilize features which have worked flawlessly for decades.  I don't
think being able to run selection converters and process filters is
poses a sufficient advantage to justify that risk.

Re: call-process should not block process filters from running

[Spencer Baugh]

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.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Wed, 28 Jun 2023 09:27:58 -0400
> Cc: emacs-devel@gnu.org, app-emacs-dev@janestreet.com
> 
> This sounds great, I would be happy to implement this.

Thanks.

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

How is this different from any other variable we bind to modify the
behavior of some API?  E.g., default-directory or
coding-system-for-read?

I really don't see a need for such trivial wrappers.

Re: call-process should not block process filters from running

[Spencer Baugh]

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.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Sat, 01 Jul 2023 14:24:59 -0400
> 
> >> 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.

We read from pipe in chunks, and my idea was to leave the reading code
intact, just call wait_reading_process_output each iteration through
the reading loop.  Long-running call-process calls spend their time in
this loop, reading the process output that takes long time to produce.
So that's where I envision most of the gains will come from.

As for waitpid, I'd initially leave that alone, and only add the
wait_reading_process_output to the loop that protects that from
signals.  IME, by the time we get to waitpid after reading the last
portion of the process output, the process have either already exited
or will do so almost instantly.  If we discover this is not so, we
could later replace waitpid with some wait with timeout in a loop.

I didn't mean to throw away the reading loop and waitpid, because
doing so will require to write a lot of non-trivial C code, and goes
against what I consider to be the main advantage of my idea.

Re: call-process should not block process filters from running

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Sat, 01 Jul 2023 14:24:59 -0400
>> 
>> >> 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.
>
> We read from pipe in chunks, and my idea was to leave the reading code
> intact, just call wait_reading_process_output each iteration through
> the reading loop.  Long-running call-process calls spend their time in
> this loop, reading the process output that takes long time to produce.
> So that's where I envision most of the gains will come from.

That's certainly better than nothing, but that won't run other Lisp
while the call-process process is not producing output.  Just as one
degenerate example, call-process on "sleep 30" won't run any Lisp.  More
realistically, expensive computations which take a while to produce
output ("factor $BIGNUM" for example) will not run much Lisp.  From
looking at call-process usage, I think most are of that form.  So I
don't think this will solve the problem.

> As for waitpid, I'd initially leave that alone, and only add the
> wait_reading_process_output to the loop that protects that from
> signals.  IME, by the time we get to waitpid after reading the last
> portion of the process output, the process have either already exited
> or will do so almost instantly.  If we discover this is not so, we
> could later replace waitpid with some wait with timeout in a loop.

I see.  But if that's the case, then that's not really any better than
just calling reading input and running other Lisp after that loop is
done, which of course we already do.  Because, if waitpid returns
immediately anyway, it doesn't really matter if we run Lisp before it or
run Lisp after it.

> I didn't mean to throw away the reading loop and waitpid, because
> doing so will require to write a lot of non-trivial C code, and goes
> against what I consider to be the main advantage of my idea.

Yes, I understand that you don't want to introduce bugs into
call-process.  IMO that's why making a completely new function instead
of modifying call-process is a better bet.

But if you would prefer a modification to call-process I'm quite happy
to write non-trivial C code, as long as the problem actually gets
solved.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Sat, 01 Jul 2023 15:17:39 -0400
> 
> > We read from pipe in chunks, and my idea was to leave the reading code
> > intact, just call wait_reading_process_output each iteration through
> > the reading loop.  Long-running call-process calls spend their time in
> > this loop, reading the process output that takes long time to produce.
> > So that's where I envision most of the gains will come from.
> 
> That's certainly better than nothing, but that won't run other Lisp
> while the call-process process is not producing output.  Just as one
> degenerate example, call-process on "sleep 30" won't run any Lisp.  More
> realistically, expensive computations which take a while to produce
> output ("factor $BIGNUM" for example) will not run much Lisp.  From
> looking at call-process usage, I think most are of that form.  So I
> don't think this will solve the problem.

My suggestion is to try first and only reject this idea if it indeed
is not useful in practice.

The "sleep 30" example is not interesting, IMO: Emacs has sleep-for
which will do the same while still reading from subprocesses and
letting timers run, so if a Lisp program calls "sleep 30" it probably
actually wants Emacs to do nothing during that time.

So IMO we should find interesting practical use cases which run
long-running programs via call-process, and see what this idea gives
us in those cases, before deciding whether it makes sense.  To see
whether the idea "holds water", it is not necessary to implement it in
its entirety: it is enough to count the number of iterations through
the loop that reads the output (perhaps also examining the effect of
making CALLPROC_BUFFER_SIZE_MIN smaller), because this will tell us
how many opportunities for calling wait_reading_process_output we will
have.

> > As for waitpid, I'd initially leave that alone, and only add the
> > wait_reading_process_output to the loop that protects that from
> > signals.  IME, by the time we get to waitpid after reading the last
> > portion of the process output, the process have either already exited
> > or will do so almost instantly.  If we discover this is not so, we
> > could later replace waitpid with some wait with timeout in a loop.
> 
> I see.  But if that's the case, then that's not really any better than
> just calling reading input and running other Lisp after that loop is
> done, which of course we already do.  Because, if waitpid returns
> immediately anyway, it doesn't really matter if we run Lisp before it or
> run Lisp after it.

That's what I'm saying, yes.  But if we find important cases where a
program sends EOF to its stdout and then proceeds with some lengthy
processing (which then forces us to wait in waitpid for a long time),
we could emulate waitpid using different APIs that wait with timeouts.

> > I didn't mean to throw away the reading loop and waitpid, because
> > doing so will require to write a lot of non-trivial C code, and goes
> > against what I consider to be the main advantage of my idea.
> 
> Yes, I understand that you don't want to introduce bugs into
> call-process.  IMO that's why making a completely new function instead
> of modifying call-process is a better bet.

It might be a better bet from the POV of not breaking call-process,
but it still runs the risk that the new API will have bugs which
call-process doesn't.  If we want the new API to be attractive, this
risk is still important and real.

> But if you would prefer a modification to call-process I'm quite happy
> to write non-trivial C code, as long as the problem actually gets
> solved.

I prefer that we try the minimal change I described, and only consider
more complicated solutions if that idea turns out to be not useful
enough.

Re: call-process should not block process filters from running

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Sat, 01 Jul 2023 15:17:39 -0400
>> 
>> > We read from pipe in chunks, and my idea was to leave the reading code
>> > intact, just call wait_reading_process_output each iteration through
>> > the reading loop.  Long-running call-process calls spend their time in
>> > this loop, reading the process output that takes long time to produce.
>> > So that's where I envision most of the gains will come from.
>> 
>> That's certainly better than nothing, but that won't run other Lisp
>> while the call-process process is not producing output.  Just as one
>> degenerate example, call-process on "sleep 30" won't run any Lisp.  More
>> realistically, expensive computations which take a while to produce
>> output ("factor $BIGNUM" for example) will not run much Lisp.  From
>> looking at call-process usage, I think most are of that form.  So I
>> don't think this will solve the problem.
>
> My suggestion is to try first and only reject this idea if it indeed
> is not useful in practice.
>
> The "sleep 30" example is not interesting, IMO: Emacs has sleep-for
> which will do the same while still reading from subprocesses and
> letting timers run, so if a Lisp program calls "sleep 30" it probably
> actually wants Emacs to do nothing during that time.
>
> So IMO we should find interesting practical use cases which run
> long-running programs via call-process, and see what this idea gives
> us in those cases, before deciding whether it makes sense.  To see
> whether the idea "holds water", it is not necessary to implement it in
> its entirety: it is enough to count the number of iterations through
> the loop that reads the output (perhaps also examining the effect of
> making CALLPROC_BUFFER_SIZE_MIN smaller), because this will tell us
> how many opportunities for calling wait_reading_process_output we will
> have.

Alright, with this trivial patch:

diff --git a/src/callproc.c b/src/callproc.c
index 6f3d4fad9be..1308228ab6c 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -778,6 +778,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 	      nread += this_read;
 	      total_read += this_read;
 
+	      wait_reading_process_output(-1, -1, 0, display_on_the_fly,
+					  NULL, NULL, 0);
 	      if (display_on_the_fly)
 		break;
 	    }

the following code (when run in the Emacs source tree) evaluates to 3
instead of 0 for me:

(length (let (marks
              (timer (run-at-time "0 sec" .1
                                  (lambda ()
                                    (push (float-time) marks)))))
          (project-find-regexp "foobar")
          (cancel-timer timer)
          marks))


which at least suggests that it really is working, and for real
functions which really use call-process.  Since project-find-regexp can
take many seconds on large repos, I think this is useful.

However, my other main test case/use case, being able to paste while
Emacs is in call-process, doesn't work with this change.  Any idea on
how to make that work?

Re: call-process should not block process filters from running

[Po Lu]

sbaugh@catern.com writes:

> However, my other main test case/use case, being able to paste while
> Emacs is in call-process, doesn't work with this change.  Any idea on
> how to make that work?

By entering the command loop or by calling `process_special_events'
every time wait_reading_process_input returns.  I don't recommend doing
either from call_process: a lot of existing code relies on Lisp code not
running inside calls to that function, and errors inside selection
handlers will be signaled even if nothing's wrong with the process being
called.

Also, you should at least check that call_process is reentrant: it must
be able to handle calls to itself made by any timers or selection
handlers that run within the process input loop, with the processes
being called completing in any order.

Re: call-process should not block process filters from running

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:
> sbaugh@catern.com writes:
>> However, my other main test case/use case, being able to paste while
>> Emacs is in call-process, doesn't work with this change.  Any idea on
>> how to make that work?
>
> By entering the command loop or by calling `process_special_events'
> every time wait_reading_process_input returns.

Thanks, with this patch:

diff --git a/src/callproc.c b/src/callproc.c
index 6f3d4fad9be..2135308c2c5 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -778,6 +778,9 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 	      nread += this_read;
 	      total_read += this_read;
 
+	      wait_reading_process_output(-1, -1, 0, display_on_the_fly,
+					  NULL, NULL, 0);
+	      swallow_events(display_on_the_fly);
 	      if (display_on_the_fly)
 		break;
 	    }

I can paste while Emacs is blocked in call_process, awesome.  (although
with some lag - but it's good enough for now)

> I don't recommend doing either from call_process: a lot of existing
> code relies on Lisp code not running inside calls to that function,
> and errors inside selection handlers will be signaled even if
> nothing's wrong with the process being called.

Right, in the real patch this will be behind a new variable which code
can let-bind to explicitly opt-in to the new behavior, as Eli suggested.
So existing code won't see behavior changes.

> Also, you should at least check that call_process is reentrant: it
> must be able to handle calls to itself made by any timers or selection
> handlers that run within the process input loop, with the processes
> being called completing in any order.

Hm, good point.  Right now in call_process there's:

  if (synch_process_pid)
    error ("call-process invoked recursively");

So it's at least "safe" to call recursively, since it explicitly errors
rather than having undefined behavior.  But erroring in this way is
probably not desirable.

I can make it re-entrant.  synch_process_pid is itself a global, so
that's the place to start.

Eli, your thoughts?  Is this more complicated than you were thinking?

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Mon, 03 Jul 2023 13:53:02 -0400
> 
> > I don't recommend doing either from call_process: a lot of existing
> > code relies on Lisp code not running inside calls to that function,
> > and errors inside selection handlers will be signaled even if
> > nothing's wrong with the process being called.
> 
> Right, in the real patch this will be behind a new variable which code
> can let-bind to explicitly opt-in to the new behavior, as Eli suggested.
> So existing code won't see behavior changes.

I'm not sure the new code should call swallow_events, even if
protected by some variable.  We will be giving people a too-long rope
to hang themselves.  It is unreasonable to assume Lisp programmers
will be able to decide when to allow this and when not to, given the
complexities.

Why do we need to jump through all the hoops right at the beginning?
Why not make a small step forward and see if this experiment is more
useful than it is dangerous?  Then we could decide whether to make the
next step, and how.  No one said that pasting into Emacs while a
subprocess runs is the most important capability to enable.  I'd first
make sure the user can type at the keyboard without adverse effects.

Let's not over-engineer this without a sufficient justification.

> > Also, you should at least check that call_process is reentrant: it
> > must be able to handle calls to itself made by any timers or selection
> > handlers that run within the process input loop, with the processes
> > being called completing in any order.
> 
> Hm, good point.  Right now in call_process there's:
> 
>   if (synch_process_pid)
>     error ("call-process invoked recursively");
> 
> So it's at least "safe" to call recursively, since it explicitly errors
> rather than having undefined behavior.  But erroring in this way is
> probably not desirable.
> 
> I can make it re-entrant.  synch_process_pid is itself a global, so
> that's the place to start.
> 
> Eli, your thoughts?  Is this more complicated than you were thinking?

I don't see how it will be more complicated to have a list or array of
PIDs instead of just one value.  This "complication" is nothing
compared to the job of rewriting call-process with all its quirks in
Lisp, even if you ignore the problems and loss of performance we'd
need to sustain to decode and insert the output in Lisp rather than in
C.

Re: call-process should not block process filters from running

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: sbaugh@catern.com
>> Date: Mon, 03 Jul 2023 13:53:02 -0400
>> 
>> > I don't recommend doing either from call_process: a lot of existing
>> > code relies on Lisp code not running inside calls to that function,
>> > and errors inside selection handlers will be signaled even if
>> > nothing's wrong with the process being called.
>> 
>> Right, in the real patch this will be behind a new variable which code
>> can let-bind to explicitly opt-in to the new behavior, as Eli suggested.
>> So existing code won't see behavior changes.
>
> I'm not sure the new code should call swallow_events, even if
> protected by some variable.  We will be giving people a too-long rope
> to hang themselves.  It is unreasonable to assume Lisp programmers
> will be able to decide when to allow this and when not to, given the
> complexities.

What is complex about this?  What problems can it cause that aren't
already covered by "arbitrary Lisp will run during call-process"?

If swallow_events is too general, I can just call only
process_special_events if necessary.

AFAIK this is a pretty small change, all it changes is letting Emacs
handle X selection requests while in call-process.  (i.e. letting other
X clients paste when Emacs owns the selection)

> Why do we need to jump through all the hoops right at the beginning?
> Why not make a small step forward and see if this experiment is more
> useful than it is dangerous?  Then we could decide whether to make the
> next step, and how.  No one said that pasting into Emacs while a
> subprocess runs is the most important capability to enable.  I'd first
> make sure the user can type at the keyboard without adverse effects.

It's not pasting into Emacs, it's pasting into other X clients.  I
indeed don't care about pasting into Emacs while a subprocess runs.  But
I like to be able to use other X clients, including by pasting, while a
subprocess runs.

This is not about allowing the user to type at the keyboard while
call-process runs - that's a much bigger change for sure!

> Let's not over-engineer this without a sufficient justification.

I've received lots of user complaints about the inability to paste in
other X clients while Emacs is sitting in call-process.  Fixing that is
one of my key motivations for this change.

>> > Also, you should at least check that call_process is reentrant: it
>> > must be able to handle calls to itself made by any timers or selection
>> > handlers that run within the process input loop, with the processes
>> > being called completing in any order.
>> 
>> Hm, good point.  Right now in call_process there's:
>> 
>>   if (synch_process_pid)
>>     error ("call-process invoked recursively");
>> 
>> So it's at least "safe" to call recursively, since it explicitly errors
>> rather than having undefined behavior.  But erroring in this way is
>> probably not desirable.
>> 
>> I can make it re-entrant.  synch_process_pid is itself a global, so
>> that's the place to start.
>> 
>> Eli, your thoughts?  Is this more complicated than you were thinking?
>
> I don't see how it will be more complicated to have a list or array of
> PIDs instead of just one value.  This "complication" is nothing
> compared to the job of rewriting call-process with all its quirks in
> Lisp, even if you ignore the problems and loss of performance we'd
> need to sustain to decode and insert the output in Lisp rather than in
> C.

OK, sounds good.

Re: call-process should not block process filters from running

[sbaugh]

The following diff makes call_process reentrant.  It can actually be
achieved quite elegantly by just moving synch_process_pid into variables
on the stack.

So with this change, this code (which executes call-process inside
call-process) works fine:

(length (let* (marks
              (timer (run-at-time "0 sec" .1
                                  (lambda ()
                                    (shell-command "true")
                                    (push (float-time) marks)))))
          (project-find-regexp "foobar")
          (cancel-timer timer)
          marks))

diff --git a/src/callproc.c b/src/callproc.c
index 6f3d4fad9be..6391ad6c6d8 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -95,26 +95,6 @@ #define _P_NOWAIT 1	/* from process.h */
 /* Pattern used by call-process-region to make temp files.  */
 static Lisp_Object Vtemp_file_name_pattern;
 
-/* The next two variables are used while record-unwind-protect is in place
-   during call-process for a subprocess for which record_deleted_pid has
-   not yet been called.  At other times, synch_process_pid is zero and
-   synch_process_tempfile's contents are irrelevant.  Doing this via static
-   C variables is more convenient than putting them into the arguments
-   of record-unwind-protect, as they need to be updated at randomish
-   times in the code, and Lisp cannot always store these values as
-   Emacs integers.  It's safe to use static variables here, as the
-   code is never invoked reentrantly.  */
-
-/* If nonzero, a process-ID that has not been reaped.  */
-static pid_t synch_process_pid;
-
-/* If a string, the name of a temp file that has not been removed.  */
-#ifdef MSDOS
-static Lisp_Object synch_process_tempfile;
-#else
-# define synch_process_tempfile make_fixnum (0)
-#endif
-
 /* Indexes of file descriptors that need closing on call_process_kill.  */
 enum
   {
@@ -189,6 +169,14 @@ record_kill_process (struct Lisp_Process *p, Lisp_Object tempfile)
 }
 
 /* Clean up files, file descriptors and processes created by Fcall_process.  */
+struct synch_process {
+  /* If nonzero, a process-ID that has not been reaped.  */
+  pid_t pid;
+  /* If a string, the name of a temp file that has not been removed.  */
+  Lisp_Object tempfile;
+  int* callproc_fd;
+  Lisp_Object buffer;
+};
 
 static void
 delete_temp_file (Lisp_Object name)
@@ -199,41 +187,43 @@ delete_temp_file (Lisp_Object name)
 static void
 call_process_kill (void *ptr)
 {
-  int *callproc_fd = ptr;
+  struct synch_process *synch_process = ptr;
+  int *callproc_fd = synch_process->callproc_fd;
   int i;
   for (i = 0; i < CALLPROC_FDS; i++)
     if (0 <= callproc_fd[i])
       emacs_close (callproc_fd[i]);
 
-  if (synch_process_pid)
+  if (synch_process->pid)
     {
       struct Lisp_Process proc;
       proc.alive = 1;
-      proc.pid = synch_process_pid;
-      record_kill_process (&proc, synch_process_tempfile);
-      synch_process_pid = 0;
+      proc.pid = synch_process->pid;
+      record_kill_process (&proc, synch_process->tempfile);
+      synch_process->pid = 0;
     }
-  else if (STRINGP (synch_process_tempfile))
-    delete_temp_file (synch_process_tempfile);
+  else if (STRINGP (synch_process->tempfile))
+    delete_temp_file (synch_process->tempfile);
 }
 
 /* Clean up when exiting Fcall_process: restore the buffer, and
    kill the subsidiary process group if the process still exists.  */
 
 static void
-call_process_cleanup (Lisp_Object buffer)
+call_process_cleanup (void *ptr)
 {
-  Fset_buffer (buffer);
+  struct synch_process *synch_process = ptr;
+  Fset_buffer (synch_process->buffer);
 
 #ifndef MSDOS
-  if (synch_process_pid)
+  if (synch_process->pid)
     {
-      kill (-synch_process_pid, SIGINT);
+      kill (-synch_process->pid, SIGINT);
       message1 ("Waiting for process to die...(type C-g again to kill it instantly)");
 
       /* This will quit on C-g.  */
-      bool wait_ok = wait_for_termination (synch_process_pid, NULL, true);
-      synch_process_pid = 0;
+      bool wait_ok = wait_for_termination (synch_process->pid, NULL, true);
+      synch_process->pid = 0;
       message1 (wait_ok
 		? "Waiting for process to die...done"
 		: "Waiting for process to die...internal error");
@@ -361,9 +351,6 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
   Lisp_Object coding_systems;
   bool discard_output;
 
-  if (synch_process_pid)
-    error ("call-process invoked recursively");
-
   /* Qt denotes that Ffind_operation_coding_system is not yet called.  */
   coding_systems = Qt;
 
@@ -489,10 +476,13 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 
   for (i = 0; i < CALLPROC_FDS; i++)
     callproc_fd[i] = -1;
-#ifdef MSDOS
-  synch_process_tempfile = make_fixnum (0);
-#endif
-  record_unwind_protect_ptr (call_process_kill, callproc_fd);
+  struct synch_process synch_process = {
+    .pid = 0,
+    .tempfile = make_fixnum (0),
+    .callproc_fd = callproc_fd,
+    .buffer = Fcurrent_buffer (),
+  };
+  record_unwind_protect_ptr (call_process_kill, &synch_process);
 
   /* Search for program; barf if not found.  */
   {
@@ -547,7 +537,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
       if (!*tempfile)
 	report_file_error ("Opening process output file", Qnil);
       output_file = build_string (tempfile);
-      synch_process_tempfile = output_file;
+      data.tempfile = output_file;
     }
 #endif
 
@@ -644,7 +634,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
   /* Do the unwind-protect now, even though the pid is not known, so
      that no storage allocation is done in the critical section.
      The actual PID will be filled in during the critical section.  */
-  record_unwind_protect (call_process_cleanup, Fcurrent_buffer ());
+  record_unwind_protect_ptr (call_process_cleanup, &synch_process);
 
 #ifndef MSDOS
 
@@ -659,7 +649,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 
   if (pid > 0)
     {
-      synch_process_pid = pid;
+      synch_process.pid = pid;
 
       if (FIXNUMP (buffer))
 	{
@@ -671,7 +661,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 	      record_deleted_pid (pid, args[1]);
 	      clear_unwind_protect (tempfile_index);
 	    }
-	  synch_process_pid = 0;
+	  synch_process.pid = 0;
 	}
     }
 
@@ -778,6 +768,9 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 	      nread += this_read;
 	      total_read += this_read;
 
+	      wait_reading_process_output(-1, -1, 0, display_on_the_fly,
+					  NULL, NULL, 0);
+	      swallow_events(display_on_the_fly);
 	      if (display_on_the_fly)
 		break;
 	    }
@@ -904,7 +897,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 
   /* Don't kill any children that the subprocess may have left behind
      when exiting.  */
-  synch_process_pid = 0;
+  synch_process.pid = 0;
 
   SAFE_FREE_UNBIND_TO (count, Qnil);
 
@@ -2035,11 +2028,6 @@ syms_of_callproc (void)
 #endif
   staticpro (&Vtemp_file_name_pattern);
 
-#ifdef MSDOS
-  synch_process_tempfile = make_fixnum (0);
-  staticpro (&synch_process_tempfile);
-#endif
-
   DEFVAR_LISP ("shell-file-name", Vshell_file_name,
 	       doc: /* File name to load inferior shells from.
 Initialized from the SHELL environment variable, or to a system-dependent

Re: call-process should not block process filters from running

[Po Lu]

sbaugh@catern.com writes:

>  /* Clean up files, file descriptors and processes created by Fcall_process.  */

This is a stray comment.

> +struct synch_process {
> +  /* If nonzero, a process-ID that has not been reaped.  */
> +  pid_t pid;
> +  /* If a string, the name of a temp file that has not been removed.  */
> +  Lisp_Object tempfile;
> +  int* callproc_fd;
> +  Lisp_Object buffer;
> +};

Please place the opening brace of this structure definition on a new
line, fill the comments above each field, and write:

  int *callproc_fd;

instead of

  int* callproc_fd;

ISTM that this field could be named `fd' instead.

> +  struct synch_process synch_process = {
> +    .pid = 0,
> +    .tempfile = make_fixnum (0),
> +    .callproc_fd = callproc_fd,
> +    .buffer = Fcurrent_buffer (),
> +  };

Why not declare `sync_process' earlier?  The code looks better that way.

> +	      wait_reading_process_output(-1, -1, 0, display_on_the_fly,
> +					  NULL, NULL, 0);
> +	      swallow_events(display_on_the_fly);

Please place a space between the function identifier and its parameter
list.

>  	      if (display_on_the_fly)
>  		break;
>  	    }
> @@ -904,7 +897,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
>  
>    /* Don't kill any children that the subprocess may have left behind
>       when exiting.  */
> -  synch_process_pid = 0;
> +  synch_process.pid = 0;
>  
>    SAFE_FREE_UNBIND_TO (count, Qnil);
>  
> @@ -2035,11 +2028,6 @@ syms_of_callproc (void)
>  #endif
>    staticpro (&Vtemp_file_name_pattern);
>  
> -#ifdef MSDOS
> -  synch_process_tempfile = make_fixnum (0);
> -  staticpro (&synch_process_tempfile);
> -#endif

Please make your modifications conditional on _not_ building the MS-DOS
port: as it is a single process ``operating system'', Emacs does not run
while a subprocess is running, so these changes are unnecessary.

I've also pointed out a general issue with this change: when a timer or
selection converter calls `call-process', it does not expect more timers
to run from within.  Assume that a timer periodically calls `rm
/var/run/foo', and it takes a few seconds for `/bin/rm' to page in,
enough for that timer to run again.  Now, one of the calls to `rm' will
fail and signal an error.

Re: call-process should not block process filters from running

[Po Lu]

sbaugh@catern.com writes:

> already covered by "arbitrary Lisp will run during call-process"?

Which is the problem.

> It's not pasting into Emacs, it's pasting into other X clients.  I
> indeed don't care about pasting into Emacs while a subprocess runs.  But
> I like to be able to use other X clients, including by pasting, while a
> subprocess runs.

The solution is to stop the subprocess before inserting the contents of
the selection into another client.

> I've received lots of user complaints about the inability to paste in
> other X clients while Emacs is sitting in call-process.  Fixing that is
> one of my key motivations for this change.

It's not a bug.  It doesn't have to be fixed.  If your particular use
case is okay with running Lisp while waiting for a process to complete,
then it should use accept-process-output, not call-process.

In the meantime, allowing timers and selection converters to run inside
call-process sounds like it will introduce many new bugs...

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Mon, 03 Jul 2023 16:28:25 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > I'm not sure the new code should call swallow_events, even if
> > protected by some variable.  We will be giving people a too-long rope
> > to hang themselves.  It is unreasonable to assume Lisp programmers
> > will be able to decide when to allow this and when not to, given the
> > complexities.
> 
> What is complex about this?

Whatever caused you to be surprised that just calling
wait_reading_process_output doesn't process selection requests.  How
many Lisp programmers understand what is going on in Emacs when such a
request is processed, and what that means for their Lisp programs?

> What problems can it cause that aren't already covered by "arbitrary
> Lisp will run during call-process"?

Processing selection requests does not really qualify as "running
arbitrary Lisp".  It is much more subtle, and I'm quite sure the
mental model of that which most Emacs users have is very simplistic.

> If swallow_events is too general, I can just call only
> process_special_events if necessary.

They are both potentially dangerous.

> AFAIK this is a pretty small change, all it changes is letting Emacs
> handle X selection requests while in call-process.  (i.e. letting other
> X clients paste when Emacs owns the selection)

Famous last words ;-)

There are no "small changes" in Emacs on this level.

> > Why do we need to jump through all the hoops right at the beginning?
> > Why not make a small step forward and see if this experiment is more
> > useful than it is dangerous?  Then we could decide whether to make the
> > next step, and how.  No one said that pasting into Emacs while a
> > subprocess runs is the most important capability to enable.  I'd first
> > make sure the user can type at the keyboard without adverse effects.
> 
> It's not pasting into Emacs, it's pasting into other X clients.  I
> indeed don't care about pasting into Emacs while a subprocess runs.  But
> I like to be able to use other X clients, including by pasting, while a
> subprocess runs.

Still not important enough, IMO, for us to have to solve this right
away, since you will have the same issue when Emacs is busy with
something else, like redrawing the display or even executing some
long-running Lisp program.

> > Let's not over-engineer this without a sufficient justification.
> 
> I've received lots of user complaints about the inability to paste in
> other X clients while Emacs is sitting in call-process.  Fixing that is
> one of my key motivations for this change.

Nothing's wrong with fixing this step by step, from where I stand.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 04 Jul 2023 12:12:44 +0800
> 
> In the meantime, allowing timers and selection converters to run inside
> call-process sounds like it will introduce many new bugs...

We will need to see about that.  If it indeed does introduce many
bugs, even though Lisp programs only turn this feature where
appropriate, we'll have to rethink this whole issue.

Re: call-process should not block process filters from running

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: sbaugh@catern.com
>> Date: Mon, 03 Jul 2023 16:28:25 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> > I'm not sure the new code should call swallow_events, even if
>> > protected by some variable.  We will be giving people a too-long rope
>> > to hang themselves.  It is unreasonable to assume Lisp programmers
>> > will be able to decide when to allow this and when not to, given the
>> > complexities.
>> 
>> What is complex about this?
>
> Whatever caused you to be surprised that just calling
> wait_reading_process_output doesn't process selection requests.  How
> many Lisp programmers understand what is going on in Emacs when such a
> request is processed, and what that means for their Lisp programs?
>
>> What problems can it cause that aren't already covered by "arbitrary
>> Lisp will run during call-process"?
>
> Processing selection requests does not really qualify as "running
> arbitrary Lisp".  It is much more subtle, and I'm quite sure the
> mental model of that which most Emacs users have is very simplistic.

I take your point, but can you be a little more concrete?  I genuinely
don't know what potential issues you're referring to that can be caused
by processing selection requests in a new place.  I'm asking just to
improve my knowledge of Emacs.

>> If swallow_events is too general, I can just call only
>> process_special_events if necessary.
>
> They are both potentially dangerous.
>
>> AFAIK this is a pretty small change, all it changes is letting Emacs
>> handle X selection requests while in call-process.  (i.e. letting other
>> X clients paste when Emacs owns the selection)
>
> Famous last words ;-)
>
> There are no "small changes" in Emacs on this level.
>
>> > Why do we need to jump through all the hoops right at the beginning?
>> > Why not make a small step forward and see if this experiment is more
>> > useful than it is dangerous?  Then we could decide whether to make the
>> > next step, and how.  No one said that pasting into Emacs while a
>> > subprocess runs is the most important capability to enable.  I'd first
>> > make sure the user can type at the keyboard without adverse effects.
>> 
>> It's not pasting into Emacs, it's pasting into other X clients.  I
>> indeed don't care about pasting into Emacs while a subprocess runs.  But
>> I like to be able to use other X clients, including by pasting, while a
>> subprocess runs.
>
> Still not important enough, IMO, for us to have to solve this right
> away, since you will have the same issue when Emacs is busy with
> something else, like redrawing the display or even executing some
> long-running Lisp program.

Redrawing the display is much quicker, and I don't use any long-running
Lisp programs.  In fact, I move long-running operations out into
asynchronous subprocesses so they can run concurrently with Emacs.  It's
only call-process usage that makes Emacs noticeably busy for me.

(As a tangent, it's kind of sad that this call-process API has ever
caused Emacs to block/stop running Lisp - subprocesses are inherently
concurrent!  They are what we use if we want Emacs to *not* block.  I
understand that that's complex, but it's just kind of ironic.)

>> > Let's not over-engineer this without a sufficient justification.
>> 
>> I've received lots of user complaints about the inability to paste in
>> other X clients while Emacs is sitting in call-process.  Fixing that is
>> one of my key motivations for this change.
>
> Nothing's wrong with fixing this step by step, from where I stand.

OK.  So what's the next step in this step-by-step from your perspective?
Should we install the option on trunk, let people use it, and gradually
make what it does broader and more fine-grained over time?

Re: call-process should not block process filters from running

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:

> sbaugh@catern.com writes:
>
>>  /* Clean up files, file descriptors and processes created by Fcall_process.  */
>
> This is a stray comment.
>
>> +struct synch_process {
>> +  /* If nonzero, a process-ID that has not been reaped.  */
>> +  pid_t pid;
>> +  /* If a string, the name of a temp file that has not been removed.  */
>> +  Lisp_Object tempfile;
>> +  int* callproc_fd;
>> +  Lisp_Object buffer;
>> +};
>
> Please place the opening brace of this structure definition on a new
> line, fill the comments above each field, and write:
>
>   int *callproc_fd;
>
> instead of
>
>   int* callproc_fd;
>
> ISTM that this field could be named `fd' instead.
>
>> +  struct synch_process synch_process = {
>> +    .pid = 0,
>> +    .tempfile = make_fixnum (0),
>> +    .callproc_fd = callproc_fd,
>> +    .buffer = Fcurrent_buffer (),
>> +  };
>
> Why not declare `sync_process' earlier?  The code looks better that way.
>
>> +	      wait_reading_process_output(-1, -1, 0, display_on_the_fly,
>> +					  NULL, NULL, 0);
>> +	      swallow_events(display_on_the_fly);
>
> Please place a space between the function identifier and its parameter
> list.
>
>>  	      if (display_on_the_fly)
>>  		break;
>>  	    }
>> @@ -904,7 +897,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
>>  
>>    /* Don't kill any children that the subprocess may have left behind
>>       when exiting.  */
>> -  synch_process_pid = 0;
>> +  synch_process.pid = 0;
>>  
>>    SAFE_FREE_UNBIND_TO (count, Qnil);
>>  
>> @@ -2035,11 +2028,6 @@ syms_of_callproc (void)
>>  #endif
>>    staticpro (&Vtemp_file_name_pattern);
>>  
>> -#ifdef MSDOS
>> -  synch_process_tempfile = make_fixnum (0);
>> -  staticpro (&synch_process_tempfile);
>> -#endif
>
> Please make your modifications conditional on _not_ building the MS-DOS
> port: as it is a single process ``operating system'', Emacs does not run
> while a subprocess is running, so these changes are unnecessary.

Thanks, I will make all these changes in next iteration of the patch.

> I've also pointed out a general issue with this change: when a timer or
> selection converter calls `call-process', it does not expect more timers
> to run from within.  Assume that a timer periodically calls `rm
> /var/run/foo', and it takes a few seconds for `/bin/rm' to page in,
> enough for that timer to run again.  Now, one of the calls to `rm' will
> fail and signal an error.

AFAIK, timers don't run concurrently with themselves.  If a timer
function is running, a second instance of that function won't run until
the first one is finished.

Re: call-process should not block process filters from running

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:
> It's not a bug.  It doesn't have to be fixed.  If your particular use
> case is okay with running Lisp while waiting for a process to complete,
> then it should use accept-process-output, not call-process.

Sure.  So I would go through Emacs, changing call-process users
case-by-case to instead use accept-process-output in a synchronous way,
if they are okay with running Lisp.  I'm fine with doing that, but
others appear to be concerned that that would hurt performance.

Also, it's worth noting that process-file makes doing that a little
tricky.  process-file already usually runs Lisp while waiting for the
process when it's using a remote file name handler, but when it's
running locally it just uses call-process.  So users of process-file
would need to be converted to start-file-process so that they're
uniformly asynchronous.  But, some TRAMP backends support process-file
but not start-file-process.  So it's not clear what a programmer should
do if they want to synchronously run a process through a file name
handler, and they're fine with Lisp running during that.  (Making
call-process optionally support running Lisp neatly solves this:
process-file users can just bind call-process-run-lisp to t around
process-file.)

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 04 Jul 2023 08:20:39 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> What is complex about this?
> >
> > Whatever caused you to be surprised that just calling
> > wait_reading_process_output doesn't process selection requests.  How
> > many Lisp programmers understand what is going on in Emacs when such a
> > request is processed, and what that means for their Lisp programs?
> >
> >> What problems can it cause that aren't already covered by "arbitrary
> >> Lisp will run during call-process"?
> >
> > Processing selection requests does not really qualify as "running
> > arbitrary Lisp".  It is much more subtle, and I'm quite sure the
> > mental model of that which most Emacs users have is very simplistic.
> 
> I take your point, but can you be a little more concrete?  I genuinely
> don't know what potential issues you're referring to that can be caused
> by processing selection requests in a new place.  I'm asking just to
> improve my knowledge of Emacs.

I never closely analyzed that, but just follow the control flow of
process_special_events and the functions it calls, and you will see
quite a few non-trivial things.

My point is that most people aren't aware of how this works, so they
will be unable to decide whether their program can or cannot handle
this kind of processing while call-process is in progress.

> > Still not important enough, IMO, for us to have to solve this right
> > away, since you will have the same issue when Emacs is busy with
> > something else, like redrawing the display or even executing some
> > long-running Lisp program.
> 
> Redrawing the display is much quicker

Except when it isn't.  E.g., with long lines, redisplay could take
several hundreds of milliseconds, and sometimes longer.

> and I don't use any long-running Lisp programs.

Maybe you don't, but others might.  "Long" here doesn't have to be
minutes or hours, btw, it could be seconds.  For example, sorting
incoming email could take that long if you've got a lot of it.

> In fact, I move long-running operations out into
> asynchronous subprocesses so they can run concurrently with Emacs.  It's
> only call-process usage that makes Emacs noticeably busy for me.

We are talking about a general-purpose API used a lot in Emacs.  We
are not talking about a feature only for your personal usage -- for
that you can do whatever you want locally.

> (As a tangent, it's kind of sad that this call-process API has ever
> caused Emacs to block/stop running Lisp - subprocesses are inherently
> concurrent!  They are what we use if we want Emacs to *not* block.  I
> understand that that's complex, but it's just kind of ironic.)

There's nothing sad or ironic about that.  call-process has its use
cases in Emacs, and they are valid ones.  Let's not exaggerate.

> OK.  So what's the next step in this step-by-step from your perspective?
> Should we install the option on trunk, let people use it, and gradually
> make what it does broader and more fine-grained over time?

I'd prefer that you run with this locally for some time, and report
back any complications or issues.  You have posted the patch, so if
someone else is interested, they can apply that locally and use it.

Re: call-process should not block process filters from running

[Po Lu]

sbaugh@catern.com writes:

> AFAIK, timers don't run concurrently with themselves.  If a timer
> function is running, a second instance of that function won't run until
> the first one is finished.

That is untrue.  Instead, we discourage timer functions that are not
reentrant from calling functions which may check for input (such as
sit-for or read-event, and notably not call-process.)

Re: call-process should not block process filters from running

[Po Lu]

sbaugh@catern.com writes:

> (As a tangent, it's kind of sad that this call-process API has ever
> caused Emacs to block/stop running Lisp - subprocesses are inherently
> concurrent!  They are what we use if we want Emacs to *not* block.  I
> understand that that's complex, but it's just kind of ironic.)

You are thinking of asynchronous subprocesses and not subprocesses
themselves.  When you run a subprocess from /bin/sh (not jsh):

#!/bin/sh

sleep 10

do you expect `sleep' to execute without suspending the parent shell?

Re: call-process should not block process filters from running

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: sbaugh@catern.com
>> Date: Tue, 04 Jul 2023 08:20:39 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> What is complex about this?
>> >
>> > Whatever caused you to be surprised that just calling
>> > wait_reading_process_output doesn't process selection requests.  How
>> > many Lisp programmers understand what is going on in Emacs when such a
>> > request is processed, and what that means for their Lisp programs?
>> >
>> >> What problems can it cause that aren't already covered by "arbitrary
>> >> Lisp will run during call-process"?
>> >
>> > Processing selection requests does not really qualify as "running
>> > arbitrary Lisp".  It is much more subtle, and I'm quite sure the
>> > mental model of that which most Emacs users have is very simplistic.
>> 
>> I take your point, but can you be a little more concrete?  I genuinely
>> don't know what potential issues you're referring to that can be caused
>> by processing selection requests in a new place.  I'm asking just to
>> improve my knowledge of Emacs.
>
> I never closely analyzed that, but just follow the control flow of
> process_special_events and the functions it calls, and you will see
> quite a few non-trivial things.
>
> My point is that most people aren't aware of how this works, so they
> will be unable to decide whether their program can or cannot handle
> this kind of processing while call-process is in progress.
>
>> > Still not important enough, IMO, for us to have to solve this right
>> > away, since you will have the same issue when Emacs is busy with
>> > something else, like redrawing the display or even executing some
>> > long-running Lisp program.
>> 
>> Redrawing the display is much quicker
>
> Except when it isn't.  E.g., with long lines, redisplay could take
> several hundreds of milliseconds, and sometimes longer.
>
>> and I don't use any long-running Lisp programs.
>
> Maybe you don't, but others might.  "Long" here doesn't have to be
> minutes or hours, btw, it could be seconds.  For example, sorting
> incoming email could take that long if you've got a lot of it.
>
>> In fact, I move long-running operations out into
>> asynchronous subprocesses so they can run concurrently with Emacs.  It's
>> only call-process usage that makes Emacs noticeably busy for me.
>
> We are talking about a general-purpose API used a lot in Emacs.  We
> are not talking about a feature only for your personal usage -- for
> that you can do whatever you want locally.

I mean as a general principle when writing Emacs Lisp.  "Do expensive
operations in separate processes" is advice I've heard and given many
times, and certainly Emacs follows it in many places.

>> OK.  So what's the next step in this step-by-step from your perspective?
>> Should we install the option on trunk, let people use it, and gradually
>> make what it does broader and more fine-grained over time?
>
> I'd prefer that you run with this locally for some time, and report
> back any complications or issues.  You have posted the patch, so if
> someone else is interested, they can apply that locally and use it.

OK, I'll install the patch I've posted at my site.  It will include the
process_special_events call, since that fixes the part that I've gotten
user complaints about.  I'll report back about how it goes in a couple
weeks.

Re: call-process should not block process filters from running

[Michael Albinus]

sbaugh@catern.com writes:

Hi,

> Also, it's worth noting that process-file makes doing that a little
> tricky.  process-file already usually runs Lisp while waiting for the
> process when it's using a remote file name handler, but when it's
> running locally it just uses call-process.  So users of process-file
> would need to be converted to start-file-process so that they're
> uniformly asynchronous.  But, some TRAMP backends support process-file
> but not start-file-process.  So it's not clear what a programmer should
> do if they want to synchronously run a process through a file name
> handler, and they're fine with Lisp running during that.  (Making
> call-process optionally support running Lisp neatly solves this:
> process-file users can just bind call-process-run-lisp to t around
> process-file.)

All of this sounds like a can of worms to Tramp. OMG. Please don't do
this change.

Best regards, Michael.

Re: call-process should not block process filters from running

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:
> sbaugh@catern.com writes:
>
>> AFAIK, timers don't run concurrently with themselves.  If a timer
>> function is running, a second instance of that function won't run until
>> the first one is finished.
>
> That is untrue.  Instead, we discourage timer functions that are not
> reentrant from calling functions which may check for input (such as
> sit-for or read-event, and notably not call-process.)

If I run

(run-at-time "0 sec" 1
             (lambda ()
               (message "in sleepy timer %s" (float-time))
               (sit-for 10)))

I get a message every 10 seconds, instead of every second.  Why is this?
I assumed it was because we don't run a single timer instance
concurrently with itself.

(info "(elisp) Timers") says

>Timer functions should also avoid calling functions that cause Emacs to
>wait, such as ‘sit-for’ (*note Waiting::).  This can lead to
>unpredictable effects, since other timers (or even the same timer) can
>run while waiting.

But it seems that the "(or even the same timer)" part is not true in
practice.  From looking at timer-event-handler this makes sense - it
looks like we remove the timer from timer-list before running it.  So
timers don't run concurrently with themselves.  Which since it's already
the case, we might as well formalize, since it's useful for Lisp
programmers...

Re: call-process should not block process filters from running

[sbaugh]

Michael Albinus <michael.albinus@gmx.de> writes:
> sbaugh@catern.com writes:
>
> Hi,
>
>> Also, it's worth noting that process-file makes doing that a little
>> tricky.  process-file already usually runs Lisp while waiting for the
>> process when it's using a remote file name handler, but when it's
>> running locally it just uses call-process.  So users of process-file
>> would need to be converted to start-file-process so that they're
>> uniformly asynchronous.  But, some TRAMP backends support process-file
>> but not start-file-process.  So it's not clear what a programmer should
>> do if they want to synchronously run a process through a file name
>> handler, and they're fine with Lisp running during that.  (Making
>> call-process optionally support running Lisp neatly solves this:
>> process-file users can just bind call-process-run-lisp to t around
>> process-file.)
>
> All of this sounds like a can of worms to Tramp. OMG. Please don't do
> this change.

There's multiple changes being discussed here.  One of them is "change
some users of process-file to use start-file-process instead".  Surely
you don't oppose that change, since it doesn't affect TRAMP internals at
all.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 04 Jul 2023 09:51:42 -0400
> 
> >Timer functions should also avoid calling functions that cause Emacs to
> >wait, such as ‘sit-for’ (*note Waiting::).  This can lead to
> >unpredictable effects, since other timers (or even the same timer) can
> >run while waiting.
> 
> But it seems that the "(or even the same timer)" part is not true in
> practice.  From looking at timer-event-handler this makes sense - it
> looks like we remove the timer from timer-list before running it.  So
> timers don't run concurrently with themselves.  Which since it's already
> the case, we might as well formalize, since it's useful for Lisp
> programmers...

You cannot formalize that because a timer function can activate the
same timer again.

Also, there's a window before timer-event-handler removes the time
during which the same timer can still run.

Basically, once you allow to run Lisp, all bets are off...

Re: call-process should not block process filters from running

[Dmitry Gutov]

On 03/07/2023 03:02, sbaugh@catern.com wrote:
> Since project-find-regexp can
> take many seconds on large repos, I think this is useful.
> 
> However, my other main test case/use case, being able to paste while
> Emacs is in call-process, doesn't work with this change.  Any idea on
> how to make that work?

Do you have some particular aim related to project-find-regexp?

Having Emacs interactive and responsive while the search is ongoing 
would require a different direction in design.

Is the goal simply to have other, unrelated code keep running?

Re: call-process should not block process filters from running

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: sbaugh@catern.com
>> Date: Tue, 04 Jul 2023 09:51:42 -0400
>> 
>> >Timer functions should also avoid calling functions that cause Emacs to
>> >wait, such as ‘sit-for’ (*note Waiting::).  This can lead to
>> >unpredictable effects, since other timers (or even the same timer) can
>> >run while waiting.
>> 
>> But it seems that the "(or even the same timer)" part is not true in
>> practice.  From looking at timer-event-handler this makes sense - it
>> looks like we remove the timer from timer-list before running it.  So
>> timers don't run concurrently with themselves.  Which since it's already
>> the case, we might as well formalize, since it's useful for Lisp
>> programmers...
>
> You cannot formalize that because a timer function can activate the
> same timer again.

Sure, but that's an explicit opt-in to running the timer again.  The
semantics could be "While a timer function is running, that timer is
deactivated; if you activate the timer in the timer function, it may run
again concurrently with the first run."

> Also, there's a window before timer-event-handler removes the time
> during which the same timer can still run.

There are many ways to get rid of that window.

Re: call-process should not block process filters from running

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 04 Jul 2023 12:53:52 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> From: sbaugh@catern.com
> >> Date: Tue, 04 Jul 2023 09:51:42 -0400
> >> 
> >> >Timer functions should also avoid calling functions that cause Emacs to
> >> >wait, such as ‘sit-for’ (*note Waiting::).  This can lead to
> >> >unpredictable effects, since other timers (or even the same timer) can
> >> >run while waiting.
> >> 
> >> But it seems that the "(or even the same timer)" part is not true in
> >> practice.  From looking at timer-event-handler this makes sense - it
> >> looks like we remove the timer from timer-list before running it.  So
> >> timers don't run concurrently with themselves.  Which since it's already
> >> the case, we might as well formalize, since it's useful for Lisp
> >> programmers...
> >
> > You cannot formalize that because a timer function can activate the
> > same timer again.
> 
> Sure, but that's an explicit opt-in to running the timer again.

The point is that someone else could opt-in to do so, out of control
of your Lisp.  That's why the manual mentions that.

> > Also, there's a window before timer-event-handler removes the time
> > during which the same timer can still run.
> 
> There are many ways to get rid of that window.

Not in Emacs Lisp, no.

Re: call-process should not block process filters from running

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 03/07/2023 03:02, sbaugh@catern.com wrote:
>> Since project-find-regexp can
>> take many seconds on large repos, I think this is useful.
>> However, my other main test case/use case, being able to paste while
>> Emacs is in call-process, doesn't work with this change.  Any idea on
>> how to make that work?
>
> Do you have some particular aim related to project-find-regexp?
>
> Having Emacs interactive and responsive while the search is ongoing
> would require a different direction in design.
>
> Is the goal simply to have other, unrelated code keep running?

I merely use project-find-regexp as an example of an important function
that uses call-process.  The goal is indeed simply to have other
unrelated code keep running.  And to be able to paste in other X
clients.  And be able to call project-find-regexp (and other
call-process using functions) from a Lisp thread.

Changing project-find-regexp to use asynchronous processes would be
nice, but more work, requiring more design effort.  Running processes
synchronously is fine, if it doesn't block unrelated code.

Re: call-process should not block process filters from running

[Michael Albinus]

sbaugh@catern.com writes:

> There's multiple changes being discussed here.  One of them is "change
> some users of process-file to use start-file-process instead".  Surely
> you don't oppose that change, since it doesn't affect TRAMP internals at
> all.

Agreed. What I'm opposing is to change call-process/process-file the
proposed way.

Best regards, Michael.

Re: call-process should not block process filters from running

[Dmitry Gutov]

On 04/07/2023 21:12, sbaugh@catern.com wrote:
> Dmitry Gutov <dmitry@gutov.dev> writes:
>> On 03/07/2023 03:02, sbaugh@catern.com wrote:
>>> Since project-find-regexp can
>>> take many seconds on large repos, I think this is useful.
>>> However, my other main test case/use case, being able to paste while
>>> Emacs is in call-process, doesn't work with this change.  Any idea on
>>> how to make that work?
>>
>> Do you have some particular aim related to project-find-regexp?
>>
>> Having Emacs interactive and responsive while the search is ongoing
>> would require a different direction in design.
>>
>> Is the goal simply to have other, unrelated code keep running?
> 
> I merely use project-find-regexp as an example of an important function
> that uses call-process.  The goal is indeed simply to have other
> unrelated code keep running.  And to be able to paste in other X
> clients.  And be able to call project-find-regexp (and other
> call-process using functions) from a Lisp thread.

Cool.

project-find-regexp also works over Tramp, though. Given Michael's 
objections in the other subthread, do you expect the proposed change 
might break that? If it simply keeps the current limitations (when 
invoked on a remote host), that's probably fine.

> Changing project-find-regexp to use asynchronous processes would be
> nice, but more work, requiring more design effort.  Running processes
> synchronously is fine, if it doesn't block unrelated code.

Sure. And there's some expected additional process handling overhead 
after switching to asynchronous calling. Though it would be nice to do 
some experiments, to measure the potential slowdown.

Re: call-process should not block process filters from running

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 04/07/2023 21:12, sbaugh@catern.com wrote:
>> Dmitry Gutov <dmitry@gutov.dev> writes:
>>> On 03/07/2023 03:02, sbaugh@catern.com wrote:
>>>> Since project-find-regexp can
>>>> take many seconds on large repos, I think this is useful.
>>>> However, my other main test case/use case, being able to paste while
>>>> Emacs is in call-process, doesn't work with this change.  Any idea on
>>>> how to make that work?
>>>
>>> Do you have some particular aim related to project-find-regexp?
>>>
>>> Having Emacs interactive and responsive while the search is ongoing
>>> would require a different direction in design.
>>>
>>> Is the goal simply to have other, unrelated code keep running?
>> I merely use project-find-regexp as an example of an important
>> function
>> that uses call-process.  The goal is indeed simply to have other
>> unrelated code keep running.  And to be able to paste in other X
>> clients.  And be able to call project-find-regexp (and other
>> call-process using functions) from a Lisp thread.
>
> Cool.
>
> project-find-regexp also works over Tramp, though. Given Michael's
> objections in the other subthread, do you expect the proposed change
> might break that? If it simply keeps the current limitations (when
> invoked on a remote host), that's probably fine.

There are a number of possible alternative approaches.  One is to change
project-find-regexp to use start-file-process instead of process-file.
This would make project-find-regexp only work with TRAMP backends that
support start-file-process, which is a loss, but there aren't actually
many such backends AFAIK.

It would be nice if Michael would respond to the actual question I
posed, which is "how to avoid this loss".

>> Changing project-find-regexp to use asynchronous processes would be
>> nice, but more work, requiring more design effort.  Running processes
>> synchronously is fine, if it doesn't block unrelated code.
>
> Sure. And there's some expected additional process handling overhead
> after switching to asynchronous calling. Though it would be nice to do
> some experiments, to measure the potential slowdown.

I suppose if project-files has a process-based implementation this is
something we'd want to do anyway, so it can be passed as stdin for
project-find-regexp.  But yes, certainly.

Re: call-process should not block process filters from running

[Michael Albinus]

sbaugh@catern.com writes:

Hi Spencer,

>> project-find-regexp also works over Tramp, though. Given Michael's
>> objections in the other subthread, do you expect the proposed change
>> might break that? If it simply keeps the current limitations (when
>> invoked on a remote host), that's probably fine.
>
> There are a number of possible alternative approaches.  One is to change
> project-find-regexp to use start-file-process instead of process-file.
> This would make project-find-regexp only work with TRAMP backends that
> support start-file-process, which is a loss, but there aren't actually
> many such backends AFAIK.
>
> It would be nice if Michael would respond to the actual question I
> posed, which is "how to avoid this loss".

I must have missed this in the thread, sorry. Could you pls repeat the
question?

Best regards, Michael.

Re: call-process should not block process filters from running

[sbaugh]

Michael Albinus <michael.albinus@gmx.de> writes:
> sbaugh@catern.com writes:
>
> Hi Spencer,
>
>>> project-find-regexp also works over Tramp, though. Given Michael's
>>> objections in the other subthread, do you expect the proposed change
>>> might break that? If it simply keeps the current limitations (when
>>> invoked on a remote host), that's probably fine.
>>
>> There are a number of possible alternative approaches.  One is to change
>> project-find-regexp to use start-file-process instead of process-file.
>> This would make project-find-regexp only work with TRAMP backends that
>> support start-file-process, which is a loss, but there aren't actually
>> many such backends AFAIK.
>>
>> It would be nice if Michael would respond to the actual question I
>> posed, which is "how to avoid this loss".
>
> I must have missed this in the thread, sorry. Could you pls repeat the
> question?
>
> Best regards, Michael.

Here we are talking about switching project-find-regexp to internally
use start-file-process instead of process-file, since
project-find-regexp can take non-trivial amounts of time, but retain the
current synchronous UI.  The benefit would be to not block other Lisp
from running while project-find-regexp runs, including in filters and
timers.  But actually running the process underlying project-find-regexp
asynchronously (e.g., having process filters and sentinels on it) is not
necessary.

However, switching from start-file-process to process-file loses support
for some TRAMP backends.  The question: Is there a way to get the
benefit (not block other Lisp from running) without the cost (losing
support for some TRAMP backends)?

Although maybe the answer is to just ensure that every TRAMP backend
supports start-file-process, since for example project-compile won't
work with out that also.

Re: call-process should not block process filters from running

[Michael Albinus]

sbaugh@catern.com writes:

Hi Spencer,

> Here we are talking about switching project-find-regexp to internally
> use start-file-process instead of process-file, since
> project-find-regexp can take non-trivial amounts of time, but retain the
> current synchronous UI.  The benefit would be to not block other Lisp
> from running while project-find-regexp runs, including in filters and
> timers.  But actually running the process underlying project-find-regexp
> asynchronously (e.g., having process filters and sentinels on it) is not
> necessary.

That you can always do, both functions are available. My concern was
rather to call start-file-process inside of process-file.

However, I must claim that Tramp has always hard times when asnychronous
calls, like timers, process sentinels, process filters and alike, invoke
other basic file operations (like file-exists-p) on remote files. These
asynchronous calls could interfer with Tramp's normal workflow on
handling such basic operations, and they are good for the famous ‘Remote
file error: Forbidden reentrant call of Tramp’ error. So I'm always
nervous if such usage patterns apply.

> However, switching from start-file-process to process-file loses support
> for some TRAMP backends.  The question: Is there a way to get the
> benefit (not block other Lisp from running) without the cost (losing
> support for some TRAMP backends)?

Well, checking the different backends (all tramp-*-file-name-handler-alist
constants), there is no Tramp backend which doesn't support process-file
and start-file-process simultaneously.

> Although maybe the answer is to just ensure that every TRAMP backend
> supports start-file-process, since for example project-compile won't
> work with out that also.

That's impossible. Some backends, like tramp-gvfs or tramp-rclone,
simply don't support remote processes.

Best regards, Michael.