From: Bruno Haible Date: Wed, 15 Oct 2003 10:19:04 +0000 (+0000) Subject: Kill subprocesses when the main process exits due to SIGHUP or SIGTERM X-Git-Tag: v0.13~212 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8baec33a32c22b01755b900f30d82e8a885f2cb4;p=thirdparty%2Fgettext.git Kill subprocesses when the main process exits due to SIGHUP or SIGTERM or similar. --- diff --git a/gettext-tools/lib/ChangeLog b/gettext-tools/lib/ChangeLog index 43ebb155d..9bb2b87fe 100644 --- a/gettext-tools/lib/ChangeLog +++ b/gettext-tools/lib/ChangeLog @@ -1,3 +1,33 @@ +2003-10-07 Bruno Haible + + More reliable subprocess cleanup. + * javacomp.c (compile_java_class): Pass slave_process = true to + execute(). create_pipe_in(), wait_subprocess(). + * javaexec.c (execute_java_class): Pass slave_process = true to + execute(). + + * wait-process.h (wait_subprocess): Add slave_process argument. + (register_slave_subprocess): New declaration. + * wait-process.c: Include string.h, signal.h, fatal-signal.h, + xmalloc.h. + (slaves_entry_t): New type. + (static_slaves, slaves, slaves_count, slaves_allocated): New variables. + (TERMINATOR): New macro. + (cleanup_slaves, register_slave_subprocess, + unregister_slave_subprocess): New functions. + (wait_subprocess): Add slave_process argument. + * execute.h (execute): Add slave_process argument. + * execute.c: Include signal.h, fatal-signal.h. + (execute): Add slave_process argument. + * pipe.h (create_pipe_in, create_pipe_out, create_pipe_bidi): Add + slave_process argument. + * pipe-in.c: Include signal.h, fatal-signal.h, wait-process.h. + (create_pipe_in): Add slave_process argument. + * pipe-out.c: Include signal.h, fatal-signal.h, wait-process.h. + (create_pipe_out): Add slave_process argument. + * pipe-bidi.c: Include signal.h, fatal-signal.h, wait-process.h. + (create_pipe_bidi): Add slave_process argument. + 2003-10-08 Bruno Haible * fatal-signal.h: New file. diff --git a/gettext-tools/lib/execute.c b/gettext-tools/lib/execute.c index debc42927..6abfc0a59 100644 --- a/gettext-tools/lib/execute.c +++ b/gettext-tools/lib/execute.c @@ -28,6 +28,7 @@ #include #include #include +#include #ifdef HAVE_UNISTD_H # include @@ -35,6 +36,7 @@ #include "error.h" #include "exit.h" +#include "fatal-signal.h" #include "wait-process.h" #include "gettext.h" @@ -109,12 +111,14 @@ nonintr_open (const char *pathname, int oflag, mode_t mode) /* Execute a command, optionally redirecting any of the three standard file descriptors to /dev/null. Return its exit code. If it didn't terminate correctly, exit if exit_on_error is true, otherwise - return 127. */ + return 127. + If slave_process is true, the child process will be terminated when its + creator receives a catchable fatal signal. */ int execute (const char *progname, const char *prog_path, char **prog_argv, bool null_stdin, bool null_stdout, bool null_stderr, - bool exit_on_error) + bool slave_process, bool exit_on_error) { #if defined _MSC_VER || defined __MINGW32__ @@ -189,8 +193,11 @@ execute (const char *progname, dependent which error is reported which way. We treat both cases as equivalent. */ #if HAVE_POSIX_SPAWN + sigset_t blocked_signals; posix_spawn_file_actions_t actions; bool actions_allocated; + posix_spawnattr_t attrs; + bool attrs_allocated; int err; pid_t child; #else @@ -198,7 +205,13 @@ execute (const char *progname, #endif #if HAVE_POSIX_SPAWN + if (slave_process) + { + sigprocmask (SIG_SETMASK, NULL, &blocked_signals); + block_fatal_signals (); + } actions_allocated = false; + attrs_allocated = false; if ((err = posix_spawn_file_actions_init (&actions)) != 0 || (actions_allocated = true, (null_stdin @@ -219,19 +232,37 @@ execute (const char *progname, "/dev/null", O_RDWR, 0)) != 0) - || (err = posix_spawnp (&child, prog_path, &actions, NULL, prog_argv, + || (slave_process + && ((err = posix_spawnattr_init (&attrs)) != 0 + || (attrs_allocated = true, + (err = posix_spawnattr_setsigmask (&attrs, + &blocked_signals)) + != 0 + || (err = posix_spawnattr_setflags (&attrs, + POSIX_SPAWN_SETSIGMASK)) + != 0))) + || (err = posix_spawnp (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, environ)) != 0)) { if (actions_allocated) posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, err, _("%s subprocess failed"), progname); return 127; } posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); #else + if (slave_process) + block_fatal_signals (); /* Use vfork() instead of fork() for efficiency. */ if ((child = vfork ()) == 0) { @@ -254,20 +285,29 @@ execute (const char *progname, || dup2 (nulloutfd, STDERR_FILENO) >= 0) && ((null_stdout && nulloutfd == STDOUT_FILENO) || (null_stderr && nulloutfd == STDERR_FILENO) - || close (nulloutfd) >= 0)))) + || close (nulloutfd) >= 0))) + && (!slave_process || (unblock_fatal_signals (), true))) execvp (prog_path, prog_argv); _exit (127); } if (child == -1) { + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, errno, _("%s subprocess failed"), progname); return 127; } #endif + if (slave_process) + { + register_slave_subprocess (child); + unblock_fatal_signals (); + } - return wait_subprocess (child, progname, null_stderr, exit_on_error); + return wait_subprocess (child, progname, null_stderr, + slave_process, exit_on_error); #endif } diff --git a/gettext-tools/lib/execute.h b/gettext-tools/lib/execute.h index 5915dc4fd..d0df8e4e5 100644 --- a/gettext-tools/lib/execute.h +++ b/gettext-tools/lib/execute.h @@ -1,5 +1,5 @@ /* Creation of autonomous subprocesses. - Copyright (C) 2001-2002 Free Software Foundation, Inc. + Copyright (C) 2001-2003 Free Software Foundation, Inc. Written by Bruno Haible , 2001. This program is free software; you can redistribute it and/or modify @@ -24,10 +24,14 @@ /* Execute a command, optionally redirecting any of the three standard file descriptors to /dev/null. Return its exit code. If it didn't terminate correctly, exit if exit_on_error is true, otherwise - return 127. */ + return 127. + If slave_process is true, the child process will be terminated when its + creator receives a catchable fatal signal. + It is recommended that no signal is blocked or ignored while execute() + is called. See pipe.h for the reason. */ extern int execute (const char *progname, const char *prog_path, char **prog_argv, bool null_stdin, bool null_stdout, bool null_stderr, - bool exit_on_error); + bool slave_process, bool exit_on_error); #endif /* _EXECUTE_H */ diff --git a/gettext-tools/lib/javacomp.c b/gettext-tools/lib/javacomp.c index 6265777b2..73afa2c15 100644 --- a/gettext-tools/lib/javacomp.c +++ b/gettext-tools/lib/javacomp.c @@ -162,7 +162,7 @@ compile_java_class (const char * const *java_sources, argv[2] = command; argv[3] = NULL; exitstatus = execute (javac, "/bin/sh", argv, false, false, false, - true); + true, true); err = (exitstatus != 0); /* Reset CLASSPATH. */ @@ -198,7 +198,8 @@ compile_java_class (const char * const *java_sources, argv[0] = "gcj"; argv[1] = "--version"; argv[2] = NULL; - child = create_pipe_in ("gcj", "gcj", argv, DEV_NULL, true, false, fd); + child = create_pipe_in ("gcj", "gcj", argv, DEV_NULL, true, true, + false, fd); gcj_present = false; if (child != -1) { @@ -235,7 +236,7 @@ compile_java_class (const char * const *java_sources, /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, "gcj", true, false); + exitstatus = wait_subprocess (child, "gcj", true, true, false); if (exitstatus != 0) gcj_present = false; } @@ -290,7 +291,8 @@ compile_java_class (const char * const *java_sources, free (command); } - exitstatus = execute ("gcj", "gcj", argv, false, false, false, true); + exitstatus = execute ("gcj", "gcj", argv, false, false, false, true, + true); err = (exitstatus != 0); /* Reset CLASSPATH. */ @@ -312,7 +314,7 @@ compile_java_class (const char * const *java_sources, argv[0] = "javac"; argv[1] = NULL; - exitstatus = execute ("javac", "javac", argv, false, true, true, + exitstatus = execute ("javac", "javac", argv, false, true, true, true, false); javac_present = (exitstatus == 0 || exitstatus == 1 || exitstatus == 2); javac_tested = true; @@ -365,7 +367,7 @@ compile_java_class (const char * const *java_sources, } exitstatus = execute ("javac", "javac", argv, false, false, false, - true); + true, true); err = (exitstatus != 0); /* Reset CLASSPATH. */ @@ -387,7 +389,7 @@ compile_java_class (const char * const *java_sources, argv[0] = "jikes"; argv[1] = NULL; - exitstatus = execute ("jikes", "jikes", argv, false, true, true, + exitstatus = execute ("jikes", "jikes", argv, false, true, true, true, false); jikes_present = (exitstatus == 0 || exitstatus == 1); jikes_tested = true; @@ -442,7 +444,7 @@ compile_java_class (const char * const *java_sources, } exitstatus = execute ("jikes", "jikes", argv, false, false, false, - true); + true, true); err = (exitstatus != 0); /* Reset CLASSPATH. */ diff --git a/gettext-tools/lib/javaexec.c b/gettext-tools/lib/javaexec.c index fcfe596fe..40f5d6ccf 100644 --- a/gettext-tools/lib/javaexec.c +++ b/gettext-tools/lib/javaexec.c @@ -206,7 +206,8 @@ execute_java_class (const char *class_name, argv[0] = "gij"; argv[1] = "--version"; argv[2] = NULL; - exitstatus = execute ("gij", "gij", argv, false, true, true, false); + exitstatus = execute ("gij", "gij", argv, false, true, true, true, + false); gij_present = (exitstatus == 0); gij_tested = true; } @@ -256,7 +257,8 @@ execute_java_class (const char *class_name, argv[0] = "java"; argv[1] = "-version"; argv[2] = NULL; - exitstatus = execute ("java", "java", argv, false, true, true, false); + exitstatus = execute ("java", "java", argv, false, true, true, true, + false); java_present = (exitstatus == 0); java_tested = true; } @@ -307,7 +309,8 @@ execute_java_class (const char *class_name, argv[0] = "jre"; argv[1] = NULL; - exitstatus = execute ("jre", "jre", argv, false, true, true, false); + exitstatus = execute ("jre", "jre", argv, false, true, true, true, + false); jre_present = (exitstatus == 0 || exitstatus == 1); jre_tested = true; } @@ -361,7 +364,7 @@ execute_java_class (const char *class_name, argv[0] = "jview"; argv[1] = "-?"; argv[2] = NULL; - exitstatus = execute ("jview", "jview", argv, false, true, true, + exitstatus = execute ("jview", "jview", argv, false, true, true, true, false); jview_present = (exitstatus == 0 || exitstatus == 1); jview_tested = true; diff --git a/gettext-tools/lib/pipe-bidi.c b/gettext-tools/lib/pipe-bidi.c index aca7ccaa4..a1db45dc1 100644 --- a/gettext-tools/lib/pipe-bidi.c +++ b/gettext-tools/lib/pipe-bidi.c @@ -27,6 +27,7 @@ #include #include #include +#include #ifdef HAVE_UNISTD_H # include @@ -34,6 +35,8 @@ #include "error.h" #include "exit.h" +#include "fatal-signal.h" +#include "wait-process.h" #include "gettext.h" #define _(str) gettext (str) @@ -102,7 +105,7 @@ pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[2]) { #if defined _MSC_VER || defined __MINGW32__ @@ -188,8 +191,11 @@ create_pipe_bidi (const char *progname, int ifd[2]; int ofd[2]; #if HAVE_POSIX_SPAWN + sigset_t blocked_signals; posix_spawn_file_actions_t actions; bool actions_allocated; + posix_spawnattr_t attrs; + bool attrs_allocated; int err; pid_t child; #else @@ -210,7 +216,13 @@ create_pipe_bidi (const char *progname, */ #if HAVE_POSIX_SPAWN + if (slave_process) + { + sigprocmask (SIG_SETMASK, NULL, &blocked_signals); + block_fatal_signals (); + } actions_allocated = false; + attrs_allocated = false; if ((err = posix_spawn_file_actions_init (&actions)) != 0 || (actions_allocated = true, (err = posix_spawn_file_actions_adddup2 (&actions, @@ -228,11 +240,26 @@ create_pipe_bidi (const char *progname, "/dev/null", O_RDWR, 0)) != 0) - || (err = posix_spawnp (&child, prog_path, &actions, NULL, prog_argv, - environ)) != 0)) + || (slave_process + && ((err = posix_spawnattr_init (&attrs)) != 0 + || (attrs_allocated = true, + (err = posix_spawnattr_setsigmask (&attrs, + &blocked_signals)) + != 0 + || (err = posix_spawnattr_setflags (&attrs, + POSIX_SPAWN_SETSIGMASK)) + != 0))) + || (err = posix_spawnp (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, + environ)) + != 0)) { if (actions_allocated) posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, err, _("%s subprocess failed"), progname); @@ -243,7 +270,11 @@ create_pipe_bidi (const char *progname, return -1; } posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); #else + if (slave_process) + block_fatal_signals (); /* Use vfork() instead of fork() for efficiency. */ if ((child = vfork ()) == 0) { @@ -260,12 +291,15 @@ create_pipe_bidi (const char *progname, || ((nulloutfd = open ("/dev/null", O_RDWR, 0)) >= 0 && (nulloutfd == STDERR_FILENO || (dup2 (nulloutfd, STDERR_FILENO) >= 0 - && close (nulloutfd) >= 0))))) + && close (nulloutfd) >= 0)))) + && (!slave_process || (unblock_fatal_signals (), true))) execvp (prog_path, prog_argv); _exit (127); } if (child == -1) { + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, errno, _("%s subprocess failed"), progname); @@ -276,6 +310,11 @@ create_pipe_bidi (const char *progname, return -1; } #endif + if (slave_process) + { + register_slave_subprocess (child); + unblock_fatal_signals (); + } close (ofd[0]); close (ifd[1]); diff --git a/gettext-tools/lib/pipe-in.c b/gettext-tools/lib/pipe-in.c index 8f9948717..c0c4b8285 100644 --- a/gettext-tools/lib/pipe-in.c +++ b/gettext-tools/lib/pipe-in.c @@ -27,6 +27,7 @@ #include #include #include +#include #ifdef HAVE_UNISTD_H # include @@ -34,6 +35,8 @@ #include "error.h" #include "exit.h" +#include "fatal-signal.h" +#include "wait-process.h" #include "gettext.h" #define _(str) gettext (str) @@ -115,7 +118,7 @@ pid_t create_pipe_in (const char *progname, const char *prog_path, char **prog_argv, const char *prog_stdin, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[1]) { #if defined _MSC_VER || defined __MINGW32__ @@ -201,8 +204,11 @@ create_pipe_in (const char *progname, /* Unix API. */ int ifd[2]; #if HAVE_POSIX_SPAWN + sigset_t blocked_signals; posix_spawn_file_actions_t actions; bool actions_allocated; + posix_spawnattr_t attrs; + bool attrs_allocated; int err; pid_t child; #else @@ -218,7 +224,13 @@ create_pipe_in (const char *progname, */ #if HAVE_POSIX_SPAWN + if (slave_process) + { + sigprocmask (SIG_SETMASK, NULL, &blocked_signals); + block_fatal_signals (); + } actions_allocated = false; + attrs_allocated = false; if ((err = posix_spawn_file_actions_init (&actions)) != 0 || (actions_allocated = true, (err = posix_spawn_file_actions_adddup2 (&actions, @@ -237,11 +249,26 @@ create_pipe_in (const char *progname, prog_stdin, O_RDONLY, 0)) != 0) - || (err = posix_spawnp (&child, prog_path, &actions, NULL, prog_argv, - environ)) != 0)) + || (slave_process + && ((err = posix_spawnattr_init (&attrs)) != 0 + || (attrs_allocated = true, + (err = posix_spawnattr_setsigmask (&attrs, + &blocked_signals)) + != 0 + || (err = posix_spawnattr_setflags (&attrs, + POSIX_SPAWN_SETSIGMASK)) + != 0))) + || (err = posix_spawnp (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, + environ)) + != 0)) { if (actions_allocated) posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, err, _("%s subprocess failed"), progname); @@ -250,7 +277,11 @@ create_pipe_in (const char *progname, return -1; } posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); #else + if (slave_process) + block_fatal_signals (); /* Use vfork() instead of fork() for efficiency. */ if ((child = vfork ()) == 0) { @@ -270,12 +301,15 @@ create_pipe_in (const char *progname, || ((stdinfd = open (prog_stdin, O_RDONLY, 0)) >= 0 && (stdinfd == STDIN_FILENO || (dup2 (stdinfd, STDIN_FILENO) >= 0 - && close (stdinfd) >= 0))))) + && close (stdinfd) >= 0)))) + && (!slave_process || (unblock_fatal_signals (), true))) execvp (prog_path, prog_argv); _exit (127); } if (child == -1) { + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, errno, _("%s subprocess failed"), progname); @@ -284,6 +318,11 @@ create_pipe_in (const char *progname, return -1; } #endif + if (slave_process) + { + register_slave_subprocess (child); + unblock_fatal_signals (); + } close (ifd[1]); fd[0] = ifd[0]; diff --git a/gettext-tools/lib/pipe-out.c b/gettext-tools/lib/pipe-out.c index 304ed1ef8..0f0450a93 100644 --- a/gettext-tools/lib/pipe-out.c +++ b/gettext-tools/lib/pipe-out.c @@ -27,6 +27,7 @@ #include #include #include +#include #ifdef HAVE_UNISTD_H # include @@ -34,6 +35,8 @@ #include "error.h" #include "exit.h" +#include "fatal-signal.h" +#include "wait-process.h" #include "gettext.h" #define _(str) gettext (str) @@ -115,7 +118,7 @@ pid_t create_pipe_out (const char *progname, const char *prog_path, char **prog_argv, const char *prog_stdout, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[1]) { #if defined _MSC_VER || defined __MINGW32__ @@ -201,8 +204,11 @@ create_pipe_out (const char *progname, /* Unix API. */ int ofd[2]; #if HAVE_POSIX_SPAWN + sigset_t blocked_signals; posix_spawn_file_actions_t actions; bool actions_allocated; + posix_spawnattr_t attrs; + bool attrs_allocated; int err; pid_t child; #else @@ -218,7 +224,13 @@ create_pipe_out (const char *progname, */ #if HAVE_POSIX_SPAWN + if (slave_process) + { + sigprocmask (SIG_SETMASK, NULL, &blocked_signals); + block_fatal_signals (); + } actions_allocated = false; + attrs_allocated = false; if ((err = posix_spawn_file_actions_init (&actions)) != 0 || (actions_allocated = true, (err = posix_spawn_file_actions_adddup2 (&actions, @@ -237,11 +249,26 @@ create_pipe_out (const char *progname, prog_stdout, O_WRONLY, 0)) != 0) - || (err = posix_spawnp (&child, prog_path, &actions, NULL, prog_argv, - environ)) != 0)) + || (slave_process + && ((err = posix_spawnattr_init (&attrs)) != 0 + || (attrs_allocated = true, + (err = posix_spawnattr_setsigmask (&attrs, + &blocked_signals)) + != 0 + || (err = posix_spawnattr_setflags (&attrs, + POSIX_SPAWN_SETSIGMASK)) + != 0))) + || (err = posix_spawnp (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, + environ)) + != 0)) { if (actions_allocated) posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, err, _("%s subprocess failed"), progname); @@ -250,7 +277,11 @@ create_pipe_out (const char *progname, return -1; } posix_spawn_file_actions_destroy (&actions); + if (attrs_allocated) + posix_spawnattr_destroy (&attrs); #else + if (slave_process) + block_fatal_signals (); /* Use vfork() instead of fork() for efficiency. */ if ((child = vfork ()) == 0) { @@ -270,12 +301,15 @@ create_pipe_out (const char *progname, || ((stdoutfd = open (prog_stdout, O_WRONLY, 0)) >= 0 && (stdoutfd == STDOUT_FILENO || (dup2 (stdoutfd, STDOUT_FILENO) >= 0 - && close (stdoutfd) >= 0))))) + && close (stdoutfd) >= 0)))) + && (!slave_process || (unblock_fatal_signals (), true))) execvp (prog_path, prog_argv); _exit (127); } if (child == -1) { + if (slave_process) + unblock_fatal_signals (); if (exit_on_error || !null_stderr) error (exit_on_error ? EXIT_FAILURE : 0, errno, _("%s subprocess failed"), progname); @@ -284,6 +318,11 @@ create_pipe_out (const char *progname, return -1; } #endif + if (slave_process) + { + register_slave_subprocess (child); + unblock_fatal_signals (); + } close (ofd[0]); fd[0] = ofd[1]; diff --git a/gettext-tools/lib/pipe.h b/gettext-tools/lib/pipe.h index e9ee75af8..ddffb55ec 100644 --- a/gettext-tools/lib/pipe.h +++ b/gettext-tools/lib/pipe.h @@ -44,13 +44,36 @@ extern "C" { After finishing communication, the caller should call wait_subprocess() to get rid of the subprocess in the process table. + If slave_process is true, the child process will be terminated when its + creator receives a catchable fatal signal or exits normally. If + slave_process is false, the child process will continue running in this + case, until it is lucky enough to attempt to communicate with its creator + and thus get a SIGPIPE signal. + If exit_on_error is false, a child process id of -1 should be treated the same way as a subprocess which accepts no input, produces no output and terminates with exit code 127. Why? Some errors during posix_spawnp() cause the function posix_spawnp() to return an error code; some other errors cause the subprocess to exit with return code 127. It is implementation dependent which error is reported which way. The caller - must treat both cases as equivalent. */ + must treat both cases as equivalent. + + It is recommended that no signal is blocked or ignored (i.e. have a + signal handler with value SIG_IGN) while any of these functions is called. + The reason is that child processes inherit the mask of blocked signals + from their parent (both through posix_spawn() and fork()/exec()); + likewise, signals ignored in the parent are also ignored in the child + (except possibly for SIGCHLD). And POSIX:2001 says [in the description + of exec()]: + "it should be noted that many existing applications wrongly + assume that they start with certain signals set to the default + action and/or unblocked. In particular, applications written + with a simpler signal model that does not include blocking of + signals, such as the one in the ISO C standard, may not behave + properly if invoked with some signals blocked. Therefore, it is + best not to block or ignore signals across execs without explicit + reason to do so, and especially not to block signals across execs + of arbitrary (not closely co-operating) programs." */ /* Open a pipe for output to a child process. * The child's stdout goes to a file. @@ -62,7 +85,7 @@ extern "C" { extern pid_t create_pipe_out (const char *progname, const char *prog_path, char **prog_argv, const char *prog_stdout, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[1]); /* Open a pipe for input from a child process. @@ -75,7 +98,7 @@ extern pid_t create_pipe_out (const char *progname, extern pid_t create_pipe_in (const char *progname, const char *prog_path, char **prog_argv, const char *prog_stdin, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[1]); /* Open a bidirectional pipe. @@ -89,7 +112,7 @@ extern pid_t create_pipe_in (const char *progname, extern pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv, bool null_stderr, - bool exit_on_error, + bool slave_process, bool exit_on_error, int fd[2]); /* The name of the "always silent" device. */ diff --git a/gettext-tools/lib/wait-process.c b/gettext-tools/lib/wait-process.c index 3d9f91a11..9a2037f4d 100644 --- a/gettext-tools/lib/wait-process.c +++ b/gettext-tools/lib/wait-process.c @@ -26,6 +26,8 @@ #include #include +#include +#include #include @@ -90,14 +92,156 @@ #include "error.h" #include "exit.h" +#include "fatal-signal.h" +#include "xmalloc.h" #include "gettext.h" #define _(str) gettext (str) +#define SIZEOF(a) (sizeof(a) / sizeof(a[0])) + +/* Type of an entry in the slaves array. + The 'used' bit determines whether this entry is currently in use. + (If pid_t was an atomic type like sig_atomic_t, we could just set the + 'child' field to 0 when unregistering a slave process, and wouldn't need + the 'used' field.) + The 'used' and 'child' fields are accessed from within the cleanup_slaves() + action, therefore we mark them as 'volatile'. */ +typedef struct +{ + volatile sig_atomic_t used; + volatile pid_t child; +} +slaves_entry_t; + +/* The registered slave subprocesses. */ +static slaves_entry_t static_slaves[32]; +static slaves_entry_t * volatile slaves = static_slaves; +static sig_atomic_t volatile slaves_count = 0; +static size_t slaves_allocated = SIZEOF (static_slaves); + +/* The termination signal for slave subprocesses. + 2003-10-07: Terminator becomes Governator. */ +#ifdef SIGHUP +# define TERMINATOR SIGHUP +#else +# define TERMINATOR SIGTERM +#endif + +/* The cleanup action. It gets called asynchronously. */ +static void +cleanup_slaves () +{ + for (;;) + { + /* Get the last registered slave. */ + size_t n = slaves_count; + if (n == 0) + break; + n--; + slaves_count = n; + /* Skip unused entries in the slaves array. */ + if (slaves[n].used) + { + pid_t slave = slaves[n].child; + + /* Kill the slave. */ + kill (slave, TERMINATOR); + } + } +} + +/* Register a subprocess as being a slave process. This means that the + subprocess will be terminated when its creator receives a catchable fatal + signal or exits normally. Registration ends when wait_subprocess() + notices that the subprocess has exited. */ +void +register_slave_subprocess (pid_t child) +{ + static bool cleanup_slaves_registered = false; + if (!cleanup_slaves_registered) + { + atexit (cleanup_slaves); + at_fatal_signal (cleanup_slaves); + cleanup_slaves_registered = true; + } + + /* Try to store the new slave in an unused entry of the slaves array. */ + { + slaves_entry_t *s = slaves; + slaves_entry_t *s_end = s + slaves_count; + + for (; s < s_end; s++) + if (!s->used) + { + /* The two uses of 'volatile' in the slaves_entry_t type above + (and ISO C 99 section 5.1.2.3.(5)) ensure that we mark the + entry as used only after the child pid has been written to the + memory location s->child. */ + s->child = child; + s->used = 1; + return; + } + } + + if (slaves_count == slaves_allocated) + { + /* Extend the slaves array. Note that we cannot use xrealloc(), + because then the cleanup_slaves() function could access an already + deallocated array. */ + slaves_entry_t *old_slaves = slaves; + size_t new_slaves_allocated = 2 * slaves_allocated; + slaves_entry_t *new_slaves = + malloc (new_slaves_allocated * sizeof (slaves_entry_t)); + if (new_slaves == NULL) + { + /* xalloc_die() will call exit() which will invoke cleanup_slaves(). + Additionally we need to kill child, because it's not yet among + the slaves list. */ + kill (child, TERMINATOR); + xalloc_die (); + } + memcpy (new_slaves, old_slaves, + slaves_allocated * sizeof (slaves_entry_t)); + slaves = new_slaves; + slaves_allocated = new_slaves_allocated; + /* Now we can free the old slaves array. */ + if (old_slaves != static_slaves) + free (old_slaves); + } + /* The three uses of 'volatile' in the types above (and ISO C 99 section + 5.1.2.3.(5)) ensure that we increment the slaves_count only after the + new slave and its 'used' bit have been written to the memory locations + that make up slaves[slaves_count]. */ + slaves[slaves_count].child = child; + slaves[slaves_count].used = 1; + slaves_count++; +} + +/* Unregister a child from the list of slave subprocesses. */ +static inline void +unregister_slave_subprocess (pid_t child) +{ + /* The easiest way to remove an entry from a list that can be used by + an asynchronous signal handler is just to mark it as unused. For this, + we rely on sig_atomic_t. */ + slaves_entry_t *s = slaves; + slaves_entry_t *s_end = s + slaves_count; + + for (; s < s_end; s++) + if (s->used && s->child == child) + s->used = 0; +} + + +/* Wait for a subprocess to finish. Return its exit code. + If it didn't terminate correctly, exit if exit_on_error is true, otherwise + return 127. */ int wait_subprocess (pid_t child, const char *progname, - bool null_stderr, bool exit_on_error) + bool null_stderr, + bool slave_process, bool exit_on_error) { /* waitpid() is just as portable as wait() nowadays. */ WAIT_T status; @@ -134,6 +278,14 @@ wait_subprocess (pid_t child, const char *progname, break; } + /* The child process has exited or was signalled. */ + + if (slave_process) + /* Unregister the child from the list of slave subprocesses, so that + later, when we exit, we don't kill a totally unrelated process which + may have acquired the same pid. */ + unregister_slave_subprocess (child); + if (WIFSIGNALED (status)) { if (exit_on_error || !null_stderr) diff --git a/gettext-tools/lib/wait-process.h b/gettext-tools/lib/wait-process.h index 7da470eaf..220014263 100644 --- a/gettext-tools/lib/wait-process.h +++ b/gettext-tools/lib/wait-process.h @@ -38,7 +38,14 @@ extern "C" { If it didn't terminate correctly, exit if exit_on_error is true, otherwise return 127. */ extern int wait_subprocess (pid_t child, const char *progname, - bool null_stderr, bool exit_on_error); + bool null_stderr, + bool slave_process, bool exit_on_error); + +/* Register a subprocess as being a slave process. This means that the + subprocess will be terminated when its creator receives a catchable fatal + signal or exits normally. Registration ends when wait_subprocess() + notices that the subprocess has exited. */ +extern void register_slave_subprocess (pid_t child); #ifdef __cplusplus diff --git a/gettext-tools/src/ChangeLog b/gettext-tools/src/ChangeLog index 3f4d08fe8..338a254b8 100644 --- a/gettext-tools/src/ChangeLog +++ b/gettext-tools/src/ChangeLog @@ -1,3 +1,19 @@ +2003-10-07 Bruno Haible + + More reliable subprocess cleanup. + * msgexec.c (process_string): Pass slave_process = true to + create_pipe_out() and wait_subprocess(). + * msgfilter.c (process_string): Pass slave_process = true to + create_pipe_bidi() and wait_subprocess(). + * msggrep.c (is_string_selected): Pass slave_process = true to + create_pipe_out() and wait_subprocess(). + * msginit.c (project_id, project_id_version, get_user_email, + language_team_address): Pass slave_process = true to create_pipe_in() + and wait_subprocess(). + * read-java.c (execute_and_read_po_output): Likewise. + * read-tcl.c (msgdomain_read_tcl): Likewise. + * urlget.c (execute_it, fetch): Pass slave_process = true to execute(). + 2003-10-05 Bruno Haible * write-java.c: Include fatal-signal.h, not signal.h. diff --git a/gettext-tools/src/msgexec.c b/gettext-tools/src/msgexec.c index 01f1066af..3d2f34474 100644 --- a/gettext-tools/src/msgexec.c +++ b/gettext-tools/src/msgexec.c @@ -330,7 +330,7 @@ process_string (const message_ty *mp, const char *str, size_t len) /* Open a pipe to a subprocess. */ child = create_pipe_out (sub_name, sub_path, sub_argv, NULL, false, true, - fd); + true, fd); if (full_write (fd[0], str, len) < len) error (EXIT_FAILURE, errno, @@ -339,7 +339,7 @@ process_string (const message_ty *mp, const char *str, size_t len) close (fd[0]); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, sub_name, false, true); + exitstatus = wait_subprocess (child, sub_name, false, true, true); if (exitcode < exitstatus) exitcode = exitstatus; } diff --git a/gettext-tools/src/msgfilter.c b/gettext-tools/src/msgfilter.c index 3f2749cee..7cdd1f05a 100644 --- a/gettext-tools/src/msgfilter.c +++ b/gettext-tools/src/msgfilter.c @@ -553,7 +553,8 @@ process_string (const char *str, size_t len, char **resultp, size_t *lengthp) int exitstatus; /* Open a bidirectional pipe to a subprocess. */ - child = create_pipe_bidi (sub_name, sub_path, sub_argv, false, true, fd); + child = create_pipe_bidi (sub_name, sub_path, sub_argv, false, true, true, + fd); /* Enable non-blocking I/O. This permits the read() and write() calls to return -1/EAGAIN without blocking; this is important for polling @@ -664,7 +665,7 @@ process_string (const char *str, size_t len, char **resultp, size_t *lengthp) close (fd[0]); /* Remove zombie process from process list. */ - exitstatus = wait_subprocess (child, sub_name, false, true); + exitstatus = wait_subprocess (child, sub_name, false, true, true); if (exitstatus != 0) error (EXIT_FAILURE, 0, _("%s subprocess terminated with exit code %d"), sub_name, exitstatus); diff --git a/gettext-tools/src/msggrep.c b/gettext-tools/src/msggrep.c index 10a55fa6d..06c9cbe8d 100644 --- a/gettext-tools/src/msggrep.c +++ b/gettext-tools/src/msggrep.c @@ -581,7 +581,7 @@ is_string_selected (int grep_pass, const char *str, size_t len) /* Open a pipe to a grep subprocess. */ child = create_pipe_out ("grep", grep_path, grep_argv[grep_pass], - DEV_NULL, false, true, fd); + DEV_NULL, false, true, true, fd); if (full_write (fd[0], str, len) < len) error (EXIT_FAILURE, errno, @@ -590,7 +590,7 @@ is_string_selected (int grep_pass, const char *str, size_t len) close (fd[0]); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, "grep", false, true); + exitstatus = wait_subprocess (child, "grep", false, true, true); return (exitstatus == 0); } else diff --git a/gettext-tools/src/msginit.c b/gettext-tools/src/msginit.c index d888ba507..9fc3a6ff8 100644 --- a/gettext-tools/src/msginit.c +++ b/gettext-tools/src/msginit.c @@ -910,7 +910,8 @@ project_id () argv[0] = "/bin/sh"; argv[1] = prog; argv[2] = NULL; - child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, false, fd); + child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, true, false, + fd); if (child == -1) goto failed; @@ -935,7 +936,7 @@ project_id () fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, prog, false, false); + exitstatus = wait_subprocess (child, prog, false, true, false); if (exitstatus != 0) { error (0, 0, _("%s subprocess failed with exit code %d"), @@ -976,7 +977,8 @@ project_id_version () argv[1] = prog; argv[2] = "yes"; argv[3] = NULL; - child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, false, fd); + child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, true, false, + fd); if (child == -1) goto failed; @@ -1001,7 +1003,7 @@ project_id_version () fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, prog, false, false); + exitstatus = wait_subprocess (child, prog, false, true, false); if (exitstatus != 0) { error (0, 0, _("%s subprocess failed with exit code %d"), @@ -1133,7 +1135,8 @@ The new message catalog should contain your email address, so that users can\n\ give you feedback about the translations, and so that maintainers can contact\n\ you in case of unexpected technical problems.\n"); argv[3] = NULL; - child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, false, fd); + child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, true, false, + fd); if (child == -1) goto failed; @@ -1158,7 +1161,7 @@ you in case of unexpected technical problems.\n"); fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, prog, false, false); + exitstatus = wait_subprocess (child, prog, false, true, false); if (exitstatus != 0) { error (0, 0, _("%s subprocess failed with exit code %d"), @@ -1214,7 +1217,8 @@ language_team_address () argv[4] = (char *) catalogname; argv[5] = (char *) language; argv[6] = NULL; - child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, false, fd); + child = create_pipe_in (prog, "/bin/sh", argv, DEV_NULL, false, true, false, + fd); if (child == -1) goto failed; @@ -1236,7 +1240,7 @@ language_team_address () fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, prog, false, false); + exitstatus = wait_subprocess (child, prog, false, true, false); if (exitstatus != 0) { error (0, 0, _("%s subprocess failed with exit code %d"), diff --git a/gettext-tools/src/read-java.c b/gettext-tools/src/read-java.c index 0fdb177b1..d015e4db4 100644 --- a/gettext-tools/src/read-java.c +++ b/gettext-tools/src/read-java.c @@ -64,7 +64,7 @@ execute_and_read_po_output (const char *progname, /* Open a pipe to the JVM. */ child = create_pipe_in (progname, prog_path, prog_argv, DEV_NULL, false, - true, fd); + true, true, fd); fp = fdopen (fd[0], "r"); if (fp == NULL) @@ -76,7 +76,7 @@ execute_and_read_po_output (const char *progname, fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, progname, false, true); + exitstatus = wait_subprocess (child, progname, false, true, true); if (exitstatus != 0) error (EXIT_FAILURE, 0, _("%s subprocess failed with exit code %d"), progname, exitstatus); diff --git a/gettext-tools/src/read-tcl.c b/gettext-tools/src/read-tcl.c index b1253bff0..a0dcf2960 100644 --- a/gettext-tools/src/read-tcl.c +++ b/gettext-tools/src/read-tcl.c @@ -100,7 +100,8 @@ msgdomain_read_tcl (const char *locale_name, const char *directory) } /* Open a pipe to the Tcl interpreter. */ - child = create_pipe_in ("tclsh", "tclsh", argv, DEV_NULL, false, true, fd); + child = create_pipe_in ("tclsh", "tclsh", argv, DEV_NULL, false, true, true, + fd); fp = fdopen (fd[0], "r"); if (fp == NULL) @@ -112,7 +113,7 @@ msgdomain_read_tcl (const char *locale_name, const char *directory) fclose (fp); /* Remove zombie process from process list, and retrieve exit status. */ - exitstatus = wait_subprocess (child, "tclsh", false, true); + exitstatus = wait_subprocess (child, "tclsh", false, true, true); if (exitstatus != 0) { if (exitstatus == 2) diff --git a/gettext-tools/src/urlget.c b/gettext-tools/src/urlget.c index 19592aa9a..700419be6 100644 --- a/gettext-tools/src/urlget.c +++ b/gettext-tools/src/urlget.c @@ -229,7 +229,8 @@ execute_it (const char *progname, { (void) private_data; - return execute (progname, prog_path, prog_argv, true, false, false, false) + return execute (progname, prog_path, prog_argv, true, false, false, true, + false) != 0; } @@ -286,7 +287,8 @@ fetch (const char *url, const char *file) argv[0] = "wget"; argv[1] = "--version"; argv[2] = NULL; - exitstatus = execute ("wget", "wget", argv, false, true, true, false); + exitstatus = execute ("wget", "wget", argv, false, true, true, true, + false); wget_present = (exitstatus == 0); wget_tested = true; } @@ -302,7 +304,8 @@ fetch (const char *url, const char *file) argv[4] = "-T"; argv[5] = "30"; argv[6] = (char *) url; argv[7] = NULL; - exitstatus = execute ("wget", "wget", argv, false, false, false, false); + exitstatus = execute ("wget", "wget", argv, false, false, false, true, + false); if (exitstatus != 127) { if (exitstatus != 0) @@ -327,7 +330,8 @@ fetch (const char *url, const char *file) argv[0] = "lynx"; argv[1] = "--version"; argv[2] = NULL; - exitstatus = execute ("lynx", "lynx", argv, false, true, true, false); + exitstatus = execute ("lynx", "lynx", argv, false, true, true, true, + false); lynx_present = (exitstatus == 0); lynx_tested = true; } @@ -341,7 +345,8 @@ fetch (const char *url, const char *file) argv[1] = "-source"; argv[2] = (char *) url; argv[3] = NULL; - exitstatus = execute ("lynx", "lynx", argv, false, false, false, false); + exitstatus = execute ("lynx", "lynx", argv, false, false, false, true, + false); if (exitstatus != 127) { if (exitstatus != 0) @@ -366,7 +371,8 @@ fetch (const char *url, const char *file) argv[0] = "curl"; argv[1] = "--version"; argv[2] = NULL; - exitstatus = execute ("curl", "curl", argv, false, true, true, false); + exitstatus = execute ("curl", "curl", argv, false, true, true, true, + false); curl_present = (exitstatus == 0 || exitstatus == 2); curl_tested = true; } @@ -380,7 +386,8 @@ fetch (const char *url, const char *file) argv[1] = "--silent"; argv[2] = (char *) url; argv[3] = NULL; - exitstatus = execute ("curl", "curl", argv, false, false, false, false); + exitstatus = execute ("curl", "curl", argv, false, false, false, true, + false); if (exitstatus != 127) { if (exitstatus != 0)