From: Bruno Haible Date: Fri, 9 Sep 2022 13:14:45 +0000 (+0200) Subject: spawn-pipe: Fix possible hangs in programs that spawn several children. X-Git-Url: https://gitweb.git.savannah.gnu.org/gitweb/?a=commitdiff_plain;h=710b168cf4d56a253d598579789a76552af6283c;p=gnulib.git spawn-pipe: Fix possible hangs in programs that spawn several children. * lib/spawn-pipe.c (create_pipe) [Unix]: Create the ifd[] and ofd[] file descriptors with the close-on-exec flag set. --- diff --git a/ChangeLog b/ChangeLog index 7f74fbea7c..c5d5788636 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2022-09-09 Bruno Haible + + spawn-pipe: Fix possible hangs in programs that spawn several children. + * lib/spawn-pipe.c (create_pipe) [Unix]: Create the ifd[] and ofd[] file + descriptors with the close-on-exec flag set. + 2022-09-09 Bruno Haible pipe-filter-gi tests: Fix long-standing failure on native Windows. diff --git a/lib/spawn-pipe.c b/lib/spawn-pipe.c index 19c1e0113e..ca279d575a 100644 --- a/lib/spawn-pipe.c +++ b/lib/spawn-pipe.c @@ -193,25 +193,24 @@ create_pipe (const char *progname, } } -#if ((defined _WIN32 && !defined __CYGWIN__) && SPAWN_PIPE_IMPL_AVOID_POSIX_SPAWN) || defined __KLIBC__ - - /* Native Windows API. - This uses _pipe(), dup2(), and _spawnv(). It could also be implemented - using the low-level functions CreatePipe(), DuplicateHandle(), - CreateProcess() and _open_osfhandle(); see the GNU make and GNU clisp - and cvs source code. */ - char *argv_mem_to_free; int ifd[2]; int ofd[2]; - int child; - int nulloutfd; - int stdinfd; - int stdoutfd; - - const char **argv = prepare_spawn (prog_argv, &argv_mem_to_free); - if (argv == NULL) - xalloc_die (); + /* It is important to create the file descriptors with the close-on-exec bit + set. + * In the child process that we are about to create here, the file + descriptors ofd[0] -> STDIN_FILENO and ifd[1] -> STDOUT_FILENO will be + preserved across exec, because each dup2 call scheduled by + posix_spawn_file_actions_adddup2 creates a file descriptor with the + close-on-exec bit clear. Similarly on native Windows, where we use + explicit DuplicateHandle calls, and on kLIBC, where we use explicit dup2 + calls. + * In the parent process, we close ofd[0] and ifd[1]; so, ofd[1] and ofd[0] + are still open. But if the parent process spawns another child process + later, if ofd[1] and ofd[0] were inherited by that child process, the + "end of input" / "end of output" detection would not work any more. The + parent or the child process would block, as long as that other child + process is running. */ if (pipe_stdout) if (pipe2_safer (ifd, O_BINARY | O_CLOEXEC) < 0) error (EXIT_FAILURE, errno, _("cannot create pipe")); @@ -227,6 +226,23 @@ create_pipe (const char *progname, * */ +#if ((defined _WIN32 && !defined __CYGWIN__) && SPAWN_PIPE_IMPL_AVOID_POSIX_SPAWN) || defined __KLIBC__ + + /* Native Windows API. + This uses _pipe(), dup2(), and _spawnv(). It could also be implemented + using the low-level functions CreatePipe(), DuplicateHandle(), + CreateProcess() and _open_osfhandle(); see the GNU make and GNU clisp + and cvs source code. */ + char *argv_mem_to_free; + int child; + int nulloutfd; + int stdinfd; + int stdoutfd; + + const char **argv = prepare_spawn (prog_argv, &argv_mem_to_free); + if (argv == NULL) + xalloc_die (); + child = -1; # if (defined _WIN32 && !defined __CYGWIN__) && SPAWN_PIPE_IMPL_AVOID_POSIX_SPAWN @@ -444,8 +460,6 @@ create_pipe (const char *progname, #else /* Unix API. */ - int ifd[2]; - int ofd[2]; sigset_t blocked_signals; posix_spawn_file_actions_t actions; bool actions_allocated; @@ -454,21 +468,6 @@ create_pipe (const char *progname, int err; pid_t child; - if (pipe_stdout) - if (pipe_safer (ifd) < 0) - error (EXIT_FAILURE, errno, _("cannot create pipe")); - if (pipe_stdin) - if (pipe_safer (ofd) < 0) - error (EXIT_FAILURE, errno, _("cannot create pipe")); -/* Data flow diagram: - * - * write system read - * parent -> ofd[1] -> ofd[0] -> child if pipe_stdin - * parent <- ifd[0] <- ifd[1] <- child if pipe_stdout - * read system write - * - */ - if (slave_process) { sigprocmask (SIG_SETMASK, NULL, &blocked_signals);