--- Begin Message ---
|
Subject: |
[PATCH 0/2] Fix spawning a child not setting standard fds properly |
|
Date: |
Mon, 27 Dec 2021 22:25:56 +0100 |
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
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly |
|
Date: |
Fri, 13 Jan 2023 16:20:28 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Hi Andrew & Josselin,
Andrew Whatson <whatson@tailcall.au> skribis:
> Thanks for your work on this, posix_spawn is a nice improvement!
>
> My comments on the changes in the wip-posix-spawn branch follow.
As discussed on IRC, I took your comments into account. I also added a
‘system*’ test based on what Josselin provided at the beginning of this
bug report (let’s not forget we were initially fixing an actual bug :-))
and updated ‘NEWS’:
527c257d6 Make 'system*' and 'piped-process' internally use 'spawn'.
551929e4f Add 'spawn'.
edfca3b7e Update gnulib to 0.1.5414-8204d and add posix_spawn, posix_spawnp.
Let me know if you notice anything fishy!
Thanks a lot, Josselin, for your hard work and for your patience.
Ludo’.
--- End Message ---