* bug#72092: popen creates "/dev/null" fds and never closes them
@ 2024-07-13 12:07 jakub-w
2024-07-13 18:22 ` bug#72092: [PATCH] fix file descriptor leak in piped_process/system*/popen/etc nathan via Bug reports for GUILE, GNU's Ubiquitous Extension Language
0 siblings, 1 reply; 2+ messages in thread
From: jakub-w @ 2024-07-13 12:07 UTC (permalink / raw)
To: 72092
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
* bug#72092: [PATCH] fix file descriptor leak in piped_process/system*/popen/etc
2024-07-13 12:07 bug#72092: popen creates "/dev/null" fds and never closes them jakub-w
@ 2024-07-13 18:22 ` nathan via Bug reports for GUILE, GNU's Ubiquitous Extension Language
0 siblings, 0 replies; 2+ messages in thread
From: nathan via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2024-07-13 18:22 UTC (permalink / raw)
To: 72092, jakub-w
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
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)
[-- Attachment #2: 0001-fix-file-descriptor-leak-in-piped_process-system-pop.patch --]
[-- Type: text/x-patch, Size: 2936 bytes --]
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(-)
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-07-13 18:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-13 12:07 bug#72092: popen creates "/dev/null" fds and never closes them jakub-w
2024-07-13 18:22 ` bug#72092: [PATCH] fix file descriptor leak in piped_process/system*/popen/etc nathan via Bug reports for GUILE, GNU's Ubiquitous Extension Language
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).