bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly

[Josselin Poiret]

Hello,

While working on the Guix installer (that needs to use system* quite a
lot), I've noticed that output/error redirection doesn't behave as
intended when combined with system*.  Here's a test you can try
at home:
--8<---------------cut here---------------start------------->8---
(call-with-output-file "/tmp/test.log"
  (lambda (port) (with-output-to-port port
                   (lambda () (with-error-to-port port
                                (lambda () (system* "bash" "-c" "echo bong 
>&2")))))))
--8<---------------cut here---------------end--------------->8---

With current Guix, you will notice that /tmp/test.log is empty,
instead of the expected "bong".

Worse even, when testing with
--8<---------------cut here---------------start------------->8---
(with-error-to-port (current-output-port) (lambda () (system* "bash" "-c" "echo 
$$; sleep 10")))
--8<---------------cut here---------------end--------------->8---
you can actually inspect `/proc/<PID>/fd/` and see that the stderr fd,
2, is actually closed.  This means that the next opened fd will take
its place, to which writes to stderr may end up.

The logic behind the stdin/out/err redirection for child processes
lies in `start_child`, in libguile/posix.c, and doesn't take into
account cases like:
* in/out/err having common values, as the common fd will be closed
before it has been dup2'd to all the std fds (which happens in the
first example);
* in/out/err having values between 0 and 2 which aren't their
corresponding std fd number, as they will not be dup2'd to
the stdin/out/err (which happens in the second example).

The first patch addresses this by:
* moving in/out/err closing logic after they've all been dup2'd;
* removing the check that in/out/err are > the corresponding
stdin/out/err;
* replacing renumber_file_descriptor by simply dup, as the former
closes fds that might be shared.  The closing logic of the first point
is enough here.

The second patch removes renumber_file_descriptor, as it is no longer
used.

Best,
Josselin

Josselin Poiret (2):
  Fix child spawning closing standard fds prematurely
  Remove unused renumber_file_descriptor

 libguile/posix.c | 72 ++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 52 deletions(-)

-- 
2.34.0