popen creates "/dev/null" fds and never closes them

  • Open
  • quality assurance status badge
Details
3 participants
  • jakub-w
  • Ludovic Courtès
  • nathan
Owner
unassigned
Submitted by
jakub-w
Severity
important

Debbugs page

jakub-w wrote 8 months ago
(address . bug-guile@gnu.org)
77cbc4b2a48a74957cc02fcc53238b29@riseup.net
Consider the following example:

(use-modules (ice-9 popen))

(parameterize ((current-error-port (%make-void-port OPEN_BOTH)))
(while #t
(close-pipe (open-pipe* OPEN_READ "free"))
(sleep 1)))

This opens a new "/dev/null" file descriptor every second and doesn't
close it when close-pipe is called.
If current ports are all fd ports, this doesn't happen.

AFAICT the problem was introduced by the commit
36fd2b4920ae926c79b936c29e739e71a6dff2bc in Guile 3.0.10.
nathan wrote 8 months ago
[PATCH] fix file descriptor leak in piped_process/system*/popen/etc
(address . 72092@debbugs.gnu.org)(address . jakub-w@riseup.net)
0ea0f005-32be-2fa1-b96a-e67d3e4ec3f6@nborghese.com
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)
From 466e4ab2c2cb1f93af481290c1d36db00976b8ce Mon Sep 17 00:00:00 2001
From: nathan <nathan_mail@nborghese.com>
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(-)

Toggle diff (90 lines)
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
Ludovic Courtès wrote 6 months ago
control message for bug #72092
(address . control@debbugs.gnu.org)
871q14n8je.fsf@gnu.org
severity 72092 important
quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 72092@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 72092
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help