This patch fixes a possible hang in programs that use the 'spawn-pipe' module and also spawn other child processes. It is also needed for fixing the hanging pipe-filter-* tests on native Windows.
2022-09-09 Bruno Haible <br...@clisp.org> 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/lib/spawn-pipe.c b/lib/spawn-pipe.c index f132624e98..ac803f48c4 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);