Guile's "sleep pipe" can leak into processes created by 'spawn'

Done

Details

4 participants

Josselin Poiret
Timo Wilken
Ludovic Courtès
Simon Tournier

Owner: unassigned

Submitted by: Timo Wilken

Severity: important

Merged with

Timo Wilken wrote on 22 Apr 2023 21:59

Crash during `guix import pypi -r'

Recipients:(address . bug-guix@gnu.org)

Message-ID:CS3JUDMNK50H.14U373XKCHRIC@lap.twilken.net

The `guix import pypi -r ...' command frequently crashes for me. When

it does, it always crashes on the third download, the one for the

first dependency's source distribution, like so:

Toggle snippet (45 lines)$ guix import pypi -r calcure

Starting download of /tmp/guix-file.7aaMby
From https://files.pythonhosted.org/packages/61/e3/c960430852d4322d42e02421690958f47fa2b42aaf4f3befb25c1c8baf21/calcure-2.8.6.tar.gz...
 …8.6.tar.gz  42KiB          2.1MiB/s 00:00 [##################] 100.0%

Starting download of /tmp/guix-file.mPnbsd
From https://files.pythonhosted.org/packages/b5/a8/8ba240c952a13b3b2bee64356dc1a1515c496100dc4c9edb6a6648927864/calcure-2.8.6-py3-none-any.whl...
 …y3-none-any.whl  49KiB     1.9MiB/s 00:00 [##################] 100.0%

Starting download of /tmp/guix-file.XzIPz0
From https://files.pythonhosted.org/packages/24/91/77edc7626400922d44019753c39e0fd911176ce3f77c3f85ac832d5d70b7/holidays-0.23.tar.gz...
In procedure connect*: Connection timed out

Starting download of /tmp/guix-file.XzIPz0
From https://web.archive.org/web/20230422212928/https://files.pythonhosted.org/packages/24/91/77edc7626400922d44019753c39e0fd911176ce3f77c3f85ac832d5d70b7/holidays-0.23.tar.gz...
In procedure connect*: Connection timed out
Trying to use Disarchive to assemble /tmp/guix-file.XzIPz0...
could not find its Disarchive specification
failed to download "/tmp/guix-file.XzIPz0" from "https://files.pythonhosted.org/packages/24/91/77edc7626400922d44019753c39e0fd911176ce3f77c3f85ac832d5d70b7/holidays-0.23.tar.gz"
Backtrace:
          10 (primitive-load "/home/timo/.config/guix/current/bin/gu…")
In guix/ui.scm:
   2300:7  9 (run-guix . _)
  2263:10  8 (run-guix-command _ . _)
In guix/scripts/import.scm:
    89:11  7 (guix-import . _)
In ice-9/boot-9.scm:
  1752:10  6 (with-exception-handler _ _ #:unwind? _ # _)
In guix/scripts/import/pypi.scm:
    97:21  5 (_)
In guix/import/utils.scm:
    638:3  4 (recursive-import _ #:repo->guix-package _ #:guix-name . #)
   600:31  3 (topological-sort _ #<procedure 7f668bf7a8d0 at guix/i…> …)
In srfi/srfi-1.scm:
   586:17  2 (map1 (("holidays" #f) ("jdatetime" #f) ("ics" #f)))
In guix/import/utils.scm:
   630:33  1 (lookup-node "holidays" #f)
In ice-9/boot-9.scm:
  1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Wrong number of values returned to continuation (expected 2)

However, that URL is valid -- running a manual `guix download'

afterwards works fine:

Toggle snippet (8 lines)

$ guix download 'https://files.pythonhosted.org/packages/24/91/77edc7626400922d44019753c39e0fd911176ce3f77c3f85ac832d5d70b7/holidays-0.23.tar.gz'

Starting download of /tmp/guix-file.vsuTGB

From https://files.pythonhosted.org/packages/24/91/77edc7626400922d44019753c39e0fd911176ce3f77c3f85ac832d5d70b7/holidays-0.23.tar.gz...

….23.tar.gz 374KiB 2.5MiB/s 00:00 [##################] 100.0%

/gnu/store/jk0v685hhwirl16la349c7i5is640181-holidays-0.23.tar.gz

0yxs1lzbqykl25yf1hcrqjb7qqw69f2zgf7991slnkmk98p1bsxw

Simon Tournier wrote on 24 Apr 2023 10:33

Recipients:

Message-ID:867cu1wufm.fsf@gmail.com

Hi,

On Sat, 22 Apr 2023 at 21:59, "Timo Wilken" <guix@twilken.net> wrote:

Toggle quote (4 lines)

> The `guix import pypi -r ...' command frequently crashes for me. When

> it does, it always crashes on the third download, the one for the

> first dependency's source distribution, like so:

Thanks for the report. I am merging with #62334 [1].

1: http://issues.guix.gnu.org/issue/62334

Cheers,

simon

zimoun wrote on 24 Apr 2023 10:28

control message for bug #63024

Recipients:(address . control@debbugs.gnu.org)

Message-ID:86bkjdwuoz.fsf@gmail.com

merge 63024 62334

quit

zimoun wrote on 24 Apr 2023 10:36

control message for bug #62765

Recipients:(address . control@debbugs.gnu.org)

Message-ID:865y9lwubt.fsf@gmail.com

merge 62765 62334

quit

Ludovic Courtès wrote on 2 May 2023 23:47

Re: bug#63024: Crash during `guix import pypi -r'

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:87jzxq5rtf.fsf@gnu.org

Hello!

Greg Hogan <code@greghogan.com> skribis:

Toggle quote (4 lines)

> Starting download of /tmp/guix-file.xReGb4

>>From https://web.archive.org/web/20230321140958/https://files.pythonhosted.org/packages/49/01/21337857631a97ab551bb2e3b5691ed6b1b9586011aa6a5355b9694a37fa/demon-2.0.6.tar.gz...

> In procedure connect*: Connection timed out

I can very much reproduce the bug with “guix import pypi -r cdlib” on

39ba8a10971f15264966823e8696d63c2995df86 and thereabouts. Turns out

it’s a fun story! (Josselin, I’m sure you’ll love it.)

First, the person who wrote ‘connect*’ *cough* hadn’t carefully read the

Guile manual, which reads (info "(guile) Ports and File Descriptors"):

Note that ‘select’ may return early for other reasons, for example due

to pending interrupts.

Clearly, ‘select’ is returning early, so we should check that and loop.

So far, so good.

Now, why is ‘select’ returning immediately? Because the underlying

select(2) call is passed an extra file descriptor, the “sleep pipe”, as

you can see in ‘scm_std_select’ in libguile, and it’s that file

descriptor, number 3, that’s active at each select(2) call:

Toggle snippet (4 lines)

28825 23:06:04 pselect6(15, [3], [14], [14], {tv_sec=10, tv_nsec=0}, NULL) = 1 (in [3], left {tv_sec=9, tv_nsec=999991063}) <0.000079>

28825 23:06:04 read(3, "A", 1) = 1 <0.000044>

Why so much activity on that “sleep pipe”, you ask? Well, once we’ve

filtered the read(3, …) calls, a pattern appears:

Toggle snippet (22 lines)  14335:28825 23:06:04 read(3, "A", 1)          = 1 <0.000044>
28825 23:06:04 read(3, "r", 1)          = 1 <0.000121>
28825 23:06:04 read(3, "c", 1)          = 1 <0.000107>
28825 23:06:04 read(3, "h", 1)          = 1 <0.000040>
28825 23:06:04 read(3, "i", 1)          = 1 <0.000049>
28825 23:06:04 read(3, "v", 1)          = 1 <0.000029>
28825 23:06:04 read(3, "e", 1)          = 1 <0.000019>
28825 23:06:04 read(3, ":", 1)          = 1 <0.000017>
28825 23:06:04 read(3, " ", 1)          = 1 <0.000019>
28825 23:06:04 read(3, " ", 1)          = 1 <0.000020>
28825 23:06:04 read(3, "/", 1)          = 1 <0.000019>
28825 23:06:04 read(3, "t", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "m", 1)          = 1 <0.000018>
28825 23:06:04 read(3, "p", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "/", 1)          = 1 <0.000016>
28825 23:06:04 read(3, "g", 1)          = 1 <0.000016>
28825 23:06:04 read(3, "u", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "i", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "x", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "-", 1)          = 1 <0.000015>

See? Who’s chatting over our sleep pipe?

Toggle snippet (24 lines)   4806:28831 23:06:03 dup2(0, 3)               = 3 <0.000011>
28831 23:06:03 dup2(3, 0)               = 0 <0.000011>
28831 23:06:03 dup2(4, 1)               = 1 <0.000012>
28831 23:06:03 dup2(5, 2)               = 2 <0.000010>
28831 23:06:03 close(3)                 = 0 <0.000010>
28831 23:06:03 close(4)                 = 0 <0.000011>
28831 23:06:03 close(5)                 = 0 <0.000010>
28831 23:06:03 close(6)                 = 0 <0.000010>
28831 23:06:03 close(7)                 = 0 <0.000010>
28831 23:06:03 close(8)                 = 0 <0.000011>
28831 23:06:03 close(9)                 = 0 <0.000010>
28831 23:06:03 close(10)                = 0 <0.000010>
28831 23:06:03 close(11)                = 0 <0.000011>
28831 23:06:03 close(12)                = 0 <0.000010>
28831 23:06:03 close(13)                = 0 <0.000010>
28831 23:06:03 close(14)                = 0 <0.000011>
28831 23:06:03 close(15)                = -1 EBADF (Bad file descriptor) <0.000011>
28831 23:06:03 prlimit64(0, RLIMIT_NOFILE, NULL, {rlim_cur=1024, rlim_max=4*1024}) = 0 <0.000011>
28831 23:06:03 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 <0.000010>
28831 23:06:03 execve("/home/ludo/src/guix/scripts/unzip", ["unzip", "/tmp/guix-file.bmholm", "-d", "/tmp/guix-directory.Ok2DAA", "cdlib-0.2.6.dist-info/METADATA"], 0x18a7db0 /* 85 v
   […]
28831 23:06:03 write(1, "Archive:  /tmp/guix-file.bmholm\n", 32) = 32 <0.000018>

‘unzip’! And all its friends invoked from (guix import pypi):

Toggle snippet (10 lines)

(call-with-temporary-directory

(lambda (dir)

(parameterize ((current-error-port (%make-void-port "rw+"))

(current-output-port (%make-void-port "rw+")))

(if (string=? "zip" (file-extension source-url))

(invoke "unzip" archive "-d" dir)

(invoke "tar" "xf" archive "-C" dir)))

…))

Looks like we shoudn’t dup(4, 1) in the child process, because 4 is the

other end of our sleep pipe. :-)

The problem exists both in 3.0.9 and Guile ‘main’. Looks related to the

file descriptor shuffling code in ‘do_spawn’. WDYT, Josselin?

Ludo’.

Ludovic Courtès wrote on 2 May 2023 23:48

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:87fs8e5rrp.fsf@gnu.org

Hello!

Greg Hogan <code@greghogan.com> skribis:

Toggle quote (4 lines)

> Starting download of /tmp/guix-file.xReGb4

>>From https://web.archive.org/web/20230321140958/https://files.pythonhosted.org/packages/49/01/21337857631a97ab551bb2e3b5691ed6b1b9586011aa6a5355b9694a37fa/demon-2.0.6.tar.gz...

> In procedure connect*: Connection timed out

I can very much reproduce the bug with “guix import pypi -r cdlib” on

39ba8a10971f15264966823e8696d63c2995df86 and thereabouts. Turns out

it’s a fun story! (Josselin, I’m sure you’ll love it.)

First, the person who wrote ‘connect*’ *cough* hadn’t carefully read the

Guile manual, which reads (info "(guile) Ports and File Descriptors"):

Note that ‘select’ may return early for other reasons, for example due

to pending interrupts.

Clearly, ‘select’ is returning early, so we should check that and loop.

So far, so good.

Now, why is ‘select’ returning immediately? Because the underlying

select(2) call is passed an extra file descriptor, the “sleep pipe”, as

you can see in ‘scm_std_select’ in libguile, and it’s that file

descriptor, number 3, that’s active at each select(2) call:

Toggle snippet (4 lines)

28825 23:06:04 pselect6(15, [3], [14], [14], {tv_sec=10, tv_nsec=0}, NULL) = 1 (in [3], left {tv_sec=9, tv_nsec=999991063}) <0.000079>

28825 23:06:04 read(3, "A", 1) = 1 <0.000044>

Why so much activity on that “sleep pipe”, you ask? Well, once we’ve

filtered the read(3, …) calls, a pattern appears:

Toggle snippet (22 lines)  14335:28825 23:06:04 read(3, "A", 1)          = 1 <0.000044>
28825 23:06:04 read(3, "r", 1)          = 1 <0.000121>
28825 23:06:04 read(3, "c", 1)          = 1 <0.000107>
28825 23:06:04 read(3, "h", 1)          = 1 <0.000040>
28825 23:06:04 read(3, "i", 1)          = 1 <0.000049>
28825 23:06:04 read(3, "v", 1)          = 1 <0.000029>
28825 23:06:04 read(3, "e", 1)          = 1 <0.000019>
28825 23:06:04 read(3, ":", 1)          = 1 <0.000017>
28825 23:06:04 read(3, " ", 1)          = 1 <0.000019>
28825 23:06:04 read(3, " ", 1)          = 1 <0.000020>
28825 23:06:04 read(3, "/", 1)          = 1 <0.000019>
28825 23:06:04 read(3, "t", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "m", 1)          = 1 <0.000018>
28825 23:06:04 read(3, "p", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "/", 1)          = 1 <0.000016>
28825 23:06:04 read(3, "g", 1)          = 1 <0.000016>
28825 23:06:04 read(3, "u", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "i", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "x", 1)          = 1 <0.000015>
28825 23:06:04 read(3, "-", 1)          = 1 <0.000015>

See? Who’s chatting over our sleep pipe?

Toggle snippet (24 lines)   4806:28831 23:06:03 dup2(0, 3)               = 3 <0.000011>
28831 23:06:03 dup2(3, 0)               = 0 <0.000011>
28831 23:06:03 dup2(4, 1)               = 1 <0.000012>
28831 23:06:03 dup2(5, 2)               = 2 <0.000010>
28831 23:06:03 close(3)                 = 0 <0.000010>
28831 23:06:03 close(4)                 = 0 <0.000011>
28831 23:06:03 close(5)                 = 0 <0.000010>
28831 23:06:03 close(6)                 = 0 <0.000010>
28831 23:06:03 close(7)                 = 0 <0.000010>
28831 23:06:03 close(8)                 = 0 <0.000011>
28831 23:06:03 close(9)                 = 0 <0.000010>
28831 23:06:03 close(10)                = 0 <0.000010>
28831 23:06:03 close(11)                = 0 <0.000011>
28831 23:06:03 close(12)                = 0 <0.000010>
28831 23:06:03 close(13)                = 0 <0.000010>
28831 23:06:03 close(14)                = 0 <0.000011>
28831 23:06:03 close(15)                = -1 EBADF (Bad file descriptor) <0.000011>
28831 23:06:03 prlimit64(0, RLIMIT_NOFILE, NULL, {rlim_cur=1024, rlim_max=4*1024}) = 0 <0.000011>
28831 23:06:03 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 <0.000010>
28831 23:06:03 execve("/home/ludo/src/guix/scripts/unzip", ["unzip", "/tmp/guix-file.bmholm", "-d", "/tmp/guix-directory.Ok2DAA", "cdlib-0.2.6.dist-info/METADATA"], 0x18a7db0 /* 85 v
   […]
28831 23:06:03 write(1, "Archive:  /tmp/guix-file.bmholm\n", 32) = 32 <0.000018>

‘unzip’! And all its friends invoked from (guix import pypi):

Toggle snippet (10 lines)

(call-with-temporary-directory

(lambda (dir)

(parameterize ((current-error-port (%make-void-port "rw+"))

(current-output-port (%make-void-port "rw+")))

(if (string=? "zip" (file-extension source-url))

(invoke "unzip" archive "-d" dir)

(invoke "tar" "xf" archive "-C" dir)))

…))

Looks like we shoudn’t dup(4, 1) in the child process, because 4 is the

other end of our sleep pipe. :-)

The problem exists both in 3.0.9 and Guile ‘main’. Looks related to the

file descriptor shuffling code in ‘do_spawn’. WDYT, Josselin?

Ludo’.

Ludovic Courtès wrote on 2 May 2023 23:49

control message for bug #63024

Recipients:(address . control@debbugs.gnu.org)

Message-ID:87a5ym5rq4.fsf@gnu.org

severity 63024 important

quit

Ludovic Courtès wrote on 2 May 2023 23:50

Recipients:(address . control@debbugs.gnu.org)

Message-ID:878re65row.fsf@gnu.org

retitle 63024 Guile's "sleep pipe" can leak into processes created by 'spawn'

quit

Ludovic Courtès wrote on 3 May 2023 11:04

Re: bug#63024: Crash during `guix import pypi -r'

Recipients:(name . Josselin Poiret)(address . dev@jpoiret.xyz)

Message-ID:87mt2l4whd.fsf@gnu.org

Hey,

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (3 lines)

> Looks like we shoudn’t dup(4, 1) in the child process, because 4 is the

> other end of our sleep pipe. :-)

How about this patch, Josselin?

Ludo’.

Toggle diff (59 lines)diff --git a/libguile/posix.c b/libguile/posix.c
index 3adc743c4..2d55d985c 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1388,11 +1388,27 @@ do_spawn (char *exec_file, char **exec_argv, char **exec_env,
     }
 
   /* Move the fds out of the way, so that duplicate fds or fds equal
-     to 0, 1, 2 don't trample each other */
+     to 0, 1, 2 don't trample each other.  Since 'system*' might give
+     us -1 for IN, OUT, or ERR, open /dev/null when that's the case.  */
+
+  if (in < 0)
+    posix_spawn_file_actions_addopen (&actions, fd_slot[0],
+                                      "/dev/null", O_RDONLY | O_CLOEXEC, 0);
+  else
+    posix_spawn_file_actions_adddup2 (&actions, in, fd_slot[0]);
+
+  if (out < 0)
+    posix_spawn_file_actions_addopen (&actions, fd_slot[1],
+                                      "/dev/null", O_WRONLY | O_CLOEXEC, 0);
+  else
+    posix_spawn_file_actions_adddup2 (&actions, out, fd_slot[1]);
+
+  if (err < 0)
+    posix_spawn_file_actions_addopen (&actions, fd_slot[2],
+                                      "/dev/null", O_WRONLY | O_CLOEXEC, 0);
+  else
+    posix_spawn_file_actions_adddup2 (&actions, err, fd_slot[2]);
 
-  posix_spawn_file_actions_adddup2 (&actions, in, fd_slot[0]);
-  posix_spawn_file_actions_adddup2 (&actions, out, fd_slot[1]);
-  posix_spawn_file_actions_adddup2 (&actions, err, fd_slot[2]);
   posix_spawn_file_actions_adddup2 (&actions, fd_slot[0], 0);
   posix_spawn_file_actions_adddup2 (&actions, fd_slot[1], 1);
   posix_spawn_file_actions_adddup2 (&actions, fd_slot[2], 2);
diff --git a/test-suite/tests/posix.test b/test-suite/tests/posix.test
index d5cf47cda..18dad8902 100644
--- a/test-suite/tests/posix.test
+++ b/test-suite/tests/posix.test
@@ -374,7 +374,17 @@
                     (system* "sh" "-c" "echo bong >&2"))))))))
 
       (and (zero? (status:exit-val status))
-           (call-with-input-file file get-string-all)))))
+           (call-with-input-file file get-string-all))))
+
+  (pass-if-equal "https://bugs.gnu.org/63024"
+      0
+    (if (file-exists? "/proc/self/fd/0")          ;on GNU/Linux?
+        (parameterize ((current-output-port (%make-void-port "w0")))
+          (system* "guile" "-c"
+                   (object->string
+                    '(exit (string=? "/dev/null"
+                                     (readlink "/proc/self/fd/1"))))))
+        (throw 'unresolved))))
 
 ;;
 ;; spawn

Simon Tournier wrote on 3 May 2023 12:08

Re: bug#62334: bug#63024: Crash during `guix import pypi -r'

Recipients:

Message-ID:86354d20e6.fsf@gmail.com

Hi Ludo,

On Wed, 03 May 2023 at 11:04, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (5 lines)

> diff --git a/libguile/posix.c b/libguile/posix.c

> index 3adc743c4..2d55d985c 100644

> --- a/libguile/posix.c

> +++ b/libguile/posix.c

Does it mean patch the current Guile or a new release of Guile?

Cheers,

simon

Ludovic Courtès wrote on 4 May 2023 13:10

Re: bug#63024: Crash during `guix import pypi -r'

Recipients:(name . Simon Tournier)(address . zimon.toutoune@gmail.com)

Message-ID:877ctotkr9.fsf_-_@gnu.org

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

Toggle quote (9 lines)

> On Wed, 03 May 2023 at 11:04, Ludovic Courtès <ludo@gnu.org> wrote:

>> diff --git a/libguile/posix.c b/libguile/posix.c

>> index 3adc743c4..2d55d985c 100644

>> --- a/libguile/posix.c

>> +++ b/libguile/posix.c

> Does it mean patch the current Guile or a new release of Guile?

It’s a patch against Guile ‘main’. That gives an incentive to push a

new Guile release.

Ludo’.

Ludovic Courtès wrote on 4 May 2023 13:13

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:87mt2ks62g.fsf_-_@gnu.org

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (8 lines)

> First, the person who wrote ‘connect*’ *cough* hadn’t carefully read the

> Guile manual, which reads (info "(guile) Ports and File Descriptors"):

> Note that ‘select’ may return early for other reasons, for example due

> to pending interrupts.

> Clearly, ‘select’ is returning early, so we should check that and loop.

Pushed a fix for that in fc6c96c88a0e4ad0b9e48272e5f97ffaa6eec36e.

Ludo’.

Ludovic Courtès wrote on 5 May 2023 10:54

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:87h6srnop6.fsf@gnu.org

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (12 lines)> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> First, the person who wrote ‘connect*’ *cough* hadn’t carefully read the
>> Guile manual, which reads (info "(guile) Ports and File Descriptors"):
>>
>>   Note that ‘select’ may return early for other reasons, for example due
>>   to pending interrupts.
>>
>> Clearly, ‘select’ is returning early, so we should check that and loop.
>
> Pushed a fix for that in fc6c96c88a0e4ad0b9e48272e5f97ffaa6eec36e.

… and updated the ‘guix’ package in

3c91f4ca490a7ac56dc0aebbca9c4bf4df201877, which should address

‘connect*’ timeout issues that were coming from ‘guix substitute’.

Ludo’.

Josselin Poiret wrote on 5 May 2023 15:39

[PATCH 2/3] Use /dev/null in piped-process if port is not backed by a fdes

Recipients:

Message-ID:269750b29a104e35a6bbd92e267130cfcf35c33e.1683293896.git.dev@jpoiret.xyz

* libguile/posix.c (piped_process): Open /dev/null to use as in/out/err

if the corresponding port is not backed by a file descriptor.

---

libguile/posix.c | 20 ++++++++++++++++----

1 file changed, 16 insertions(+), 4 deletions(-)

Toggle diff (33 lines)diff --git a/libguile/posix.c b/libguile/posix.c
index 2969f1f24..f41a99efe 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1558,10 +1558,22 @@ 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);
-    if (out == -1 && SCM_OPOUTFPORTP ((port = scm_current_output_port ())))
-      out = SCM_FPORT_FDES (port);
-    if (in == -1 && SCM_OPINFPORTP ((port = scm_current_input_port ())))
-      in = SCM_FPORT_FDES (port);
+    else
+      err = open ("/dev/null", O_WRONLY | O_CLOEXEC);
+    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);
+      }
+    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);
+      }
   }
 
   *pid = do_spawn (exec_file, exec_argv, exec_env, in, out, err, 1);
-- 
2.39.2

Josselin Poiret wrote on 5 May 2023 15:39

[PATCH 3/3] tests: Test that system* works if stdin/out/err isn't backed by fdes

Recipients:

Message-ID:19baf4a40b5ce28624a7bcb3f9381fce0f318cf3.1683293896.git.dev@jpoiret.xyz

* test-suite/tests/posix.test: New test for https://bugs.gnu.org/63024.

---

test-suite/tests/posix.test | 12 +++++++++++-

1 file changed, 11 insertions(+), 1 deletion(-)

Toggle diff (25 lines)diff --git a/test-suite/tests/posix.test b/test-suite/tests/posix.test
index d5cf47cda..18dad8902 100644
--- a/test-suite/tests/posix.test
+++ b/test-suite/tests/posix.test
@@ -374,7 +374,17 @@
                     (system* "sh" "-c" "echo bong >&2"))))))))
 
       (and (zero? (status:exit-val status))
-           (call-with-input-file file get-string-all)))))
+           (call-with-input-file file get-string-all))))
+
+  (pass-if-equal "https://bugs.gnu.org/63024"
+      0
+    (if (file-exists? "/proc/self/fd/0")          ;on GNU/Linux?
+        (parameterize ((current-output-port (%make-void-port "w0")))
+          (system* "guile" "-c"
+                   (object->string
+                    '(exit (string=? "/dev/null"
+                                     (readlink "/proc/self/fd/1"))))))
+        (throw 'unresolved))))
 
 ;;
 ;; spawn
-- 
2.39.2

Josselin Poiret wrote on 5 May 2023 15:39

[PATCH 1/3] Add error handling for spawn's posix_spawn_file_actions_adddup2

Recipients:

Message-ID:f7db538ea324545f2e297b6a6cdcd937f889ef63.1683293896.git.dev@jpoiret.xyz

* libguile/posix.c (do_spawn): Add error handling if

posix_spawn_file_actions_adddup2 fails.

---

Hi Ludo,

Sorry for not noticing this thread sooner! Here's my take on it, a minor

variation but that's what I'd prefer tbh.

LMKWYT,

Josselin

libguile/posix.c | 15 +++++++++------

1 file changed, 9 insertions(+), 6 deletions(-)

Toggle diff (30 lines)diff --git a/libguile/posix.c b/libguile/posix.c
index 3adc743c4..2969f1f24 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1390,12 +1390,15 @@ do_spawn (char *exec_file, char **exec_argv, char **exec_env,
   /* Move the fds out of the way, so that duplicate fds or fds equal
      to 0, 1, 2 don't trample each other */
 
-  posix_spawn_file_actions_adddup2 (&actions, in, fd_slot[0]);
-  posix_spawn_file_actions_adddup2 (&actions, out, fd_slot[1]);
-  posix_spawn_file_actions_adddup2 (&actions, err, fd_slot[2]);
-  posix_spawn_file_actions_adddup2 (&actions, fd_slot[0], 0);
-  posix_spawn_file_actions_adddup2 (&actions, fd_slot[1], 1);
-  posix_spawn_file_actions_adddup2 (&actions, fd_slot[2], 2);
+  int dup2_action_from[] = {in, out, err,
+                            fd_slot[0], fd_slot[1], fd_slot[2]};
+  int dup2_action_to  [] = {fd_slot[0], fd_slot[1], fd_slot[2],
+                            0, 1, 2};
+
+  errno = 0;
+  for (int i = 0;i < sizeof (dup2_action_from) / sizeof (int);i++)
+    if ((errno = posix_spawn_file_actions_adddup2 (&actions, dup2_action_from[i], dup2_action_to[i])))
+        return -1;
 
 #ifdef HAVE_ADDCLOSEFROM
   /* This function appears in glibc 2.34.  It's both free from race

base-commit: fe6cc6d04ab094ea802907bdc4f728416c0e97ba
-- 
2.39.2

Ludovic Courtès wrote on 8 May 2023 16:08

Re: bug#63024: Guile's "sleep pipe" can leak into processes created by 'spawn'

Recipients:(name . Josselin Poiret)(address . dev@jpoiret.xyz)

Message-ID:87mt2edifn.fsf_-_@gnu.org

Hi Josselin,

Josselin Poiret <dev@jpoiret.xyz> skribis:

Toggle quote (3 lines)

> Sorry for not noticing this thread sooner! Here's my take on it, a minor

> variation but that's what I'd prefer tbh.

Even nicer! Pushed to ‘main’ as two patches (I folded the test along

with the fix, for clarity) and with cosmetic changes to match GNU coding

style for C.

36fd2b492 main origin/main Use /dev/null in 'piped-process' if port is not backed by a fdes.

ccd7400fd Add error handling for spawn's posix_spawn_file_actions_adddup2.

Thanks!

Ludo’.

Ludovic Courtès wrote on 6 Sep 22:57 +0200

control message for bug #63024

Recipients:(address . control@debbugs.gnu.org)

Message-ID:8734mcxzye.fsf@gnu.org

close 63024

quit

Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it

mumi current 63024

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

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date