bug#56002: src/process.c; make-process fails to clean up stderr process on early exit

[Tom Gillespie]

After digging deep into this I think I have a solution. It is attached
as two patches, one that adds tests and one that updates Fmake_proc.
For the record, there was not a missing unwind-protect.

There are two changes to the implementation that are complementary.

The first is to change the internal structure of Fmake_process so that
stderrproc is created immediately before the call to create_process.
This avoids the need to call remove_process on stderrproc in the event
of any expected or unexpected exits prior to the call to create process.

This resolves the issue as originally described.

The second change is to decouple the query-on-exit behavior of
stderrproc from that of proc by adding a new keyword argument
:query-stderr which defaults to nil. The coupling of query-on-exit for
proc and stderrproc in the currently implementation forces users to
always add additional boilerplate specifically when they don't care
about stderr, which is the opposite of what would want (forcing
callers who don't care about something to handle it, rather than the
other way around).

In addition to the patches, I have a question about an inconsistency
in the documentation, and a suggestion about how it might be resolved.
I can open this as a new issue if that makes more sense.

One thing I don't understand is the interaction between :command
'(nil) and :stderr a-pipe-process. It seems that the behaviors
described by their documentation should be mutually exclusive, but
make-process creates a pty without complaining.

I suggest that either an error be raised if :command '(nil) and
:stderr a-pipe-process are provided at the same time, because in all
other cases for :stderr there is a call to CHECK_STRING that will
cause an error, and it is not clear why the pipe process variant should
be exempt from this check since it does not appear that the stderr pipe
process is ever hooked up to the pty.

Any insights here would be appreciated.

At the very least the documentation for :connection-type needs to be
updated to correct the following, since it does not match the current
implemented behavior

> if a non-nil value is specified for the :stderr parameter ... the type will 
> always be pipe

As it stands I think I have preserved existing behavior with some
slight code duplication that reflects the confusion about what should
happens when a stderr process is provided and create_pty is called
instead of create_process.

0001-test-src-process-tests.el-test-make-process-stderr-q.patch
Description: Text Data

0002-make-process-stderrproc-cleanup-on-failure-decouple-.patch
Description: Text Data