unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
From: Josselin Poiret via "Bug reports for GUILE, GNU's Ubiquitous Extension Language" <bug-guile@gnu.org>
To: 52835@debbugs.gnu.org
Cc: Josselin Poiret <dev@jpoiret.xyz>
Subject: bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly
Date: Mon, 27 Dec 2021 22:25:56 +0100	[thread overview]
Message-ID: <cover.1640638819.git.dev@jpoiret.xyz> (raw)

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






             reply	other threads:[~2021-12-27 21:25 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-27 21:25 Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language [this message]
2021-12-27 21:35 ` bug#52835: [PATCH 1/2] Fix child spawning closing standard fds prematurely Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-12-27 21:35 ` bug#52835: [PATCH 2/2] Remove unused renumber_file_descriptor Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-12-27 21:49   ` bug#52835: [PATCH v2 " Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-12-28 15:40 ` bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Timothy Sample
2021-12-28 17:25   ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-02-07 16:55     ` bug#52835: [PATCH v3] Fix child spawning closing standard fds prematurely Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-05-28 12:46       ` bug#52835: [PATCH v4 0/4] Improve safety of start_child and piped-process Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-05-28 12:46         ` bug#52835: [PATCH v4 1/4] Fix child spawning closing standard fds prematurely Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-05-28 12:46         ` bug#52835: [PATCH v4 2/4] Avoid double closes in piped-process Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-05-28 12:46         ` bug#52835: [PATCH v4 3/4] Remove useless closing code in start_child Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-05-28 12:46         ` bug#52835: [PATCH v4 4/4] Make start_child propagate the child errno to the parent Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-09-05  6:48         ` bug#52835: [PATCH v5 0/3] Move spawning procedures to posix_spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-09-05  6:48           ` bug#52835: [PATCH v5 1/3] Update gnulib to 0.1.5414-8204d and add posix_spawn, posix_spawnp Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-09-05  6:48           ` bug#52835: [PATCH v5 2/3] Add spawn* Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-09-05  6:48           ` bug#52835: [PATCH v5 3/3] Move popen and posix procedures to spawn* Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-11-29 15:05             ` bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Ludovic Courtès
2022-12-11 20:16               ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-12 23:49                 ` Ludovic Courtès
2022-12-22 12:49                   ` bug#52835: [PATCH v6 0/3] Move spawning procedures to posix_spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-22 12:49                     ` bug#52835: [PATCH v6 1/3] Add spawn* Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-22 12:49                     ` bug#52835: [PATCH v6 2/3] Make system* and piped-process internally use spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-22 12:49                     ` bug#52835: [PATCH v6 3/3] Move popen and posix procedures to spawn* Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-23 10:53                     ` bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Ludovic Courtès
2022-12-23 17:15                       ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-23 17:17                         ` bug#52835: [PATCH v7 1/2] Add spawn* and spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-23 17:17                           ` bug#52835: [PATCH v7 2/2] Make system* and piped-process internally use spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2022-12-25 17:04                             ` bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Ludovic Courtès
2022-12-25 17:03                           ` Ludovic Courtès
2022-12-25 16:58                         ` Ludovic Courtès
2023-01-07 16:07                           ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-01-07 16:07                             ` bug#52835: [PATCH v8 1/2] Add spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-01-07 16:07                               ` bug#52835: [PATCH v8 2/2] Make system* and piped-process internally use spawn Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-01-12 22:02                             ` bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Ludovic Courtès
2023-01-13  1:11 ` Andrew Whatson via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-01-13 15:20   ` Ludovic Courtès

Reply instructions:

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

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

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

  List information: https://www.gnu.org/software/guile/

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

  git send-email \
    --in-reply-to=cover.1640638819.git.dev@jpoiret.xyz \
    --to=bug-guile@gnu.org \
    --cc=52835@debbugs.gnu.org \
    --cc=dev@jpoiret.xyz \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).