unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: 61095@debbugs.gnu.org
Cc: "Ludovic Courtès" <ludo@gnu.org>,
	"Josselin Poiret" <dev@jpoiret.xyz>,
	"Omar Polo" <op@omarpolo.com>,
	"Andrew Whatson" <whatson@tailcall.au>
Subject: bug#61095: [PATCH 2/3] Remove racy optimized file descriptor closing loop in 'spawn'.
Date: Thu, 30 Mar 2023 00:30:56 +0200	[thread overview]
Message-ID: <20230329223057.28100-2-ludo@gnu.org> (raw)
In-Reply-To: <20230329223057.28100-1-ludo@gnu.org>

This reverts 9332b632407894c2e1951cce1bc678f19e1fa8e4, thereby
reinstating the performance issue in <https://bugs.gnu.org/59321>.

This optimization was subject to race conditions in multi-threaded code:
new file descriptors could pop up at any time and thus leak in the
child.

* libguile/posix.c (close_inherited_fds): Remove.
(close_inherited_fds_slow): Rename to...
(close_inherited_fds): ... this.
---
 libguile/posix.c | 30 +-----------------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/libguile/posix.c b/libguile/posix.c
index 68e9bfade..b5830c43b 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1323,7 +1323,7 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0,
 #endif /* HAVE_FORK */
 
 static void
-close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd)
+close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd)
 {
   while (--max_fd > 2)
     {
@@ -1346,34 +1346,6 @@ close_inherited_fds_slow (posix_spawn_file_actions_t *actions, int max_fd)
     }
 }
 
-static void
-close_inherited_fds (posix_spawn_file_actions_t *actions, int max_fd)
-{
-  DIR *dirp;
-  struct dirent *d;
-  int fd;
-
-  /* Try to use the platform-specific list of open file descriptors, so
-     we don't need to use the brute force approach. */
-  dirp = opendir ("/proc/self/fd");
-
-  if (dirp == NULL)
-    return close_inherited_fds_slow (actions, max_fd);
-
-  while ((d = readdir (dirp)) != NULL)
-    {
-      fd = atoi (d->d_name);
-
-      /* Skip "." and "..", garbage entries, stdin/stdout/stderr. */
-      if (fd <= 2)
-        continue;
-
-      posix_spawn_file_actions_addclose (actions, fd);
-    }
-
-  closedir (dirp);
-}
-
 static pid_t
 do_spawn (char *exec_file, char **exec_argv, char **exec_env,
           int in, int out, int err, int spawnp)
-- 
2.39.2






  reply	other threads:[~2023-03-29 22:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-27 11:51 bug#61095: possible misuse of posix_spawn API on non-linux OSes Omar Polo
2023-01-27 12:25 ` Omar Polo
2023-03-28  9:34 ` Ludovic Courtès
2023-03-28 16:10   ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-03-29 22:30     ` Ludovic Courtès
2023-03-29 22:30       ` bug#61095: [PATCH 1/3] 'spawn' closes only open file descriptors on non-GNU/Linux systems Ludovic Courtès
2023-03-29 22:30         ` Ludovic Courtès [this message]
2023-03-29 22:30         ` bug#61095: [PATCH 3/3] Use 'posix_spawn_file_actions_addclosefrom_np' where available Ludovic Courtès
2023-03-30 20:21       ` bug#61095: possible misuse of posix_spawn API on non-linux OSes Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2023-03-31 17:45         ` Omar Polo
2023-04-02 13:44           ` 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=20230329223057.28100-2-ludo@gnu.org \
    --to=ludo@gnu.org \
    --cc=61095@debbugs.gnu.org \
    --cc=dev@jpoiret.xyz \
    --cc=op@omarpolo.com \
    --cc=whatson@tailcall.au \
    /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).