bug#72092: [PATCH] fix file descriptor leak in piped_process/system*/popen/etc

nathan via "Bug reports for GUILE, GNU's Ubiquitous Extension Language" <[email protected]>
Newsgroups gmane.lisp.guile.bugs
Message-ID <[email protected]>
fix is attached

test code:
(use-modules (ice-9 ftw))
(use-modules (ice-9 popen))
(define (print-open)
    (format #t "~a\n" (length (scandir "/proc/self/fd"))))
(let ((p (%make-void-port "rw")))
    (print-open)
    (parameterize ((current-output-port p) (current-input-port p) (current-error-port p))
      (system* "ls"))
    (close-port p)
    (print-open))
(parameterize ((current-error-port (%make-void-port OPEN_BOTH)))
      (while
       #t
       (close-pipe (open-pipe* OPEN_READ "free"))
       (print-open)
       (sleep 1)))

there's still a one-time increase in the file descriptors, but it seems unrelated. or at least much harder to find.

(by the way, ignoring current-output-port and using /dev/null is bad. it would be nice to open a pipe in that case and forward whatever the child writes to the scheme port.
but that would require a separate thread, and it may be impossible to do something like that for current-input-port)
0001-fix-file-descriptor-leak-in-piped_process-system-pop.patch (text/x-patch, 2.9 KB)
From 466e4ab2c2cb1f93af481290c1d36db00976b8ce Mon Sep 17 00:00:00 2001
From: nathan <[email protected]>
Date: Sat, 13 Jul 2024 13:25:08 -0400
Subject: [PATCH] fix file descriptor leak in piped_process/system*/popen/etc

* libguile/posix.c (piped_process): close automatically opened /dev/null
  Fix bug #72092
---
 libguile/posix.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/libguile/posix.c b/libguile/posix.c
index 9a873b5a1..8028b493f 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1535,32 +1535,32 @@ static int
 piped_process (pid_t *pid, SCM prog, SCM args, SCM from, SCM to)
 #define FUNC_NAME "piped-process"
 {
-  int reading, writing;
   int c2p[2]; /* Child to parent.  */
   int p2c[2]; /* Parent to child.  */
   int in = -1, out = -1, err = -1;
   char *exec_file;
   char **exec_argv;
   char **exec_env = environ;
+  int to_close[3];
+  int num_to_close=0;
 
   exec_file = scm_to_locale_string (prog);
   exec_argv = scm_i_allocate_string_pointers (scm_cons (prog, args));
 
-  reading = scm_is_pair (from);
-  writing = scm_is_pair (to);
-
-  if (reading)
+  if (scm_is_pair (from))
     {
       c2p[0] = scm_to_int (scm_car (from));
       c2p[1] = scm_to_int (scm_cdr (from));
       out = c2p[1];
+      to_close[num_to_close++] = out;
     }
 
-  if (writing)
+  if (scm_is_pair (to))
     {
       p2c[0] = scm_to_int (scm_car (to));
       p2c[1] = scm_to_int (scm_cdr (to));
       in = p2c[0];
+      to_close[num_to_close++] = in;
     }
 
   {
@@ -1569,30 +1569,37 @@ piped_process (pid_t *pid, SCM prog, SCM args, SCM from, SCM to)
     if (SCM_OPOUTFPORTP ((port = scm_current_error_port ())))
       err = SCM_FPORT_FDES (port);
     else
-      err = open ("/dev/null", O_WRONLY | O_CLOEXEC);
+      {
+        err = open ("/dev/null", O_WRONLY | O_CLOEXEC);
+        to_close[num_to_close++] = err;
+      }
     if (out == -1)
       {
         if (SCM_OPOUTFPORTP ((port = scm_current_output_port ())))
           out = SCM_FPORT_FDES (port);
         else
-          out = open ("/dev/null", O_WRONLY | O_CLOEXEC);
+          {
+            out = open ("/dev/null", O_WRONLY | O_CLOEXEC);
+            to_close[num_to_close++] = out;
+          }
       }
     if (in == -1)
       {
         if (SCM_OPINFPORTP ((port = scm_current_input_port ())))
           in = SCM_FPORT_FDES (port);
         else
-          in = open ("/dev/null", O_RDONLY | O_CLOEXEC);
+          {
+            in = open ("/dev/null", O_RDONLY | O_CLOEXEC);
+            to_close[num_to_close++] = in;
+          }
       }
   }
 
   *pid = do_spawn (exec_file, exec_argv, exec_env, in, out, err, 1);
   int errno_save = (*pid < 0) ? errno : 0;
 
-  if (reading)
-    close (c2p[1]);
-  if (writing)
-    close (p2c[0]);
+  while (num_to_close>0)
+    close (to_close[--num_to_close]);
 
   if (*pid == -1)
     switch (errno_save)
-- 
2.45.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.