From 22bae83fa8c432780fe20202a660aa8c84f3087a Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 17 Nov 2012 18:46:45 +0200 Subject: [PATCH] Fix bug #12829 with aborts on MS-Windows when several child processes die. nt/inc/sys/wait.h: New file, with prototype of waitpid and definitions of macros it needs. nt/inc/ms-w32.h (wait): Don't define, 'wait' is not used anymore. (sys_wait): Remove prototype. nt/config.nt (HAVE_SYS_WAIT_H): Define to 1. src/w32proc.c (create_child): Don't clip the PID of the child process to fit into an Emacs integer, as this is no longer a restriction. (waitpid): Rename from sys_wait. Emulate a Posix 'waitpid' by reaping only the process specified by PID argument, if that is positive. Use PID instead of dead_child to know which process to reap. Wait for the child to die only if WNOHANG is not in OPTIONS. (sys_select): Don't set dead_child. src/sysdep.c (wait_for_termination_1): Remove the WINDOWSNT portion, as it is no longer needed. src/process.c (waitpid, WUNTRACED) [!WNOHANG]: Remove definitions, no longer needed. (record_child_status_change): Remove the setting of record_at_most_one_child for the !WNOHANG case. --- nt/ChangeLog | 10 ++++ nt/config.nt | 2 +- nt/inc/ms-w32.h | 3 -- nt/inc/sys/wait.h | 33 ++++++++++++ src/ChangeLog | 20 ++++++++ src/process.c | 20 -------- src/sysdep.c | 7 +-- src/w32proc.c | 127 ++++++++++++++++++++++++++++++++-------------- 8 files changed, 156 insertions(+), 66 deletions(-) create mode 100644 nt/inc/sys/wait.h diff --git a/nt/ChangeLog b/nt/ChangeLog index aa690e5d75f..b24acae8be5 100644 --- a/nt/ChangeLog +++ b/nt/ChangeLog @@ -1,3 +1,13 @@ +2012-11-17 Eli Zaretskii + + * inc/sys/wait.h: New file, with prototype of waitpid and + definitions of macros it needs. + + * inc/ms-w32.h (wait): Don't define, 'wait' is not used anymore. + (sys_wait): Remove prototype. + + * config.nt (HAVE_SYS_WAIT_H): Define to 1. + 2012-11-17 Dani Moncayo * zipdist.bat (ZIP_CHECK): Remove unused label. When invoking 7z diff --git a/nt/config.nt b/nt/config.nt index 69549fb2087..638f0a7461b 100644 --- a/nt/config.nt +++ b/nt/config.nt @@ -986,7 +986,7 @@ along with GNU Emacs. If not, see . */ #undef HAVE_SYS_VLIMIT_H /* Define to 1 if you have that is POSIX.1 compatible. */ -#undef HAVE_SYS_WAIT_H +#define HAVE_SYS_WAIT_H 1 /* Define to 1 if you have the header file. */ #undef HAVE_TERM_H diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h index 1b2a309e1be..7b16ccab069 100644 --- a/nt/inc/ms-w32.h +++ b/nt/inc/ms-w32.h @@ -183,15 +183,12 @@ extern char *getenv (); /* Subprocess calls that are emulated. */ #define spawnve sys_spawnve -#define wait sys_wait #define kill sys_kill #define signal sys_signal /* Internal signals. */ #define emacs_raise(sig) emacs_abort() -extern int sys_wait (int *); - /* termcap.c calls that are emulated. */ #define tputs sys_tputs #define tgetstr sys_tgetstr diff --git a/nt/inc/sys/wait.h b/nt/inc/sys/wait.h new file mode 100644 index 00000000000..8d890c9e175 --- /dev/null +++ b/nt/inc/sys/wait.h @@ -0,0 +1,33 @@ +/* A limited emulation of sys/wait.h on Posix systems. + +Copyright (C) 2012 Free Software Foundation, Inc. + +This file is part of GNU Emacs. + +GNU Emacs is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +GNU Emacs is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GNU Emacs. If not, see . */ + +#ifndef INC_SYS_WAIT_H_ +#define INC_SYS_WAIT_H_ + +#define WNOHANG 1 +#define WUNTRACED 2 +#define WSTOPPED 2 /* same as WUNTRACED */ +#define WEXITED 4 +#define WCONTINUED 8 + +/* The various WIF* macros are defined in src/syswait.h. */ + +extern pid_t waitpid (pid_t, int *, int); + +#endif /* INC_SYS_WAIT_H_ */ diff --git a/src/ChangeLog b/src/ChangeLog index 1194fe099fa..45d48ba41cc 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,23 @@ +2012-11-17 Eli Zaretskii + + * w32proc.c (create_child): Don't clip the PID of the child + process to fit into an Emacs integer, as this is no longer a + restriction. + (waitpid): Rename from sys_wait. Emulate a Posix 'waitpid' by + reaping only the process specified by PID argument, if that is + positive. Use PID instead of dead_child to know which process to + reap. Wait for the child to die only if WNOHANG is not in + OPTIONS. + (sys_select): Don't set dead_child. + + * sysdep.c (wait_for_termination_1): Remove the WINDOWSNT portion, + as it is no longer needed. + + * process.c (waitpid, WUNTRACED) [!WNOHANG]: Remove definitions, + no longer needed. + (record_child_status_change): Remove the setting of + record_at_most_one_child for the !WNOHANG case. + 2012-11-17 Paul Eggert Fix problems in ns port found by static checking. diff --git a/src/process.c b/src/process.c index 785282fba36..5fe6a6540f3 100644 --- a/src/process.c +++ b/src/process.c @@ -130,18 +130,6 @@ extern int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *, EMACS_TIME *, void *); #endif -/* This is for DOS_NT ports. FIXME: Remove this old portability cruft - by having DOS_NT ports implement waitpid instead of wait. Nowadays - POSIXish hosts all define waitpid, WNOHANG, and WUNTRACED, as these - have been standard since POSIX.1-1988. */ -#ifndef WNOHANG -# undef waitpid -# define waitpid(pid, status, options) wait (status) -#endif -#ifndef WUNTRACED -# define WUNTRACED 0 -#endif - /* Work around GCC 4.7.0 bug with strict overflow checking; see . These lines can be removed once the GCC bug is fixed. */ @@ -6295,17 +6283,9 @@ record_child_status_change (pid_t pid, int w) { #ifdef SIGCHLD -# ifdef WNOHANG /* On POSIXish hosts, record at most one child only if we already know one child that has exited. */ bool record_at_most_one_child = 0 <= pid; -# else - /* On DOS_NT (the only porting target that lacks WNOHANG), - record the status of at most one child process, since the SIGCHLD - handler must return right away. If any more processes want to - signal us, we will get another signal. */ - bool record_at_most_one_child = 1; -# endif Lisp_Object tail; diff --git a/src/sysdep.c b/src/sysdep.c index a7f3de2f1b1..06dc41b511e 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -289,10 +289,6 @@ wait_for_termination_1 (pid_t pid, int interruptible) { while (1) { -#ifdef WINDOWSNT - wait (0); - break; -#else /* not WINDOWSNT */ int status; int wait_result = waitpid (pid, &status, 0); if (wait_result < 0) @@ -306,7 +302,8 @@ wait_for_termination_1 (pid_t pid, int interruptible) break; } -#endif /* not WINDOWSNT */ + /* Note: the MS-Windows emulation of waitpid calls QUIT + internally. */ if (interruptible) QUIT; } diff --git a/src/w32proc.c b/src/w32proc.c index 10dd23003b8..fd6a498290a 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -789,7 +789,6 @@ alarm (int seconds) /* Child process management list. */ int child_proc_count = 0; child_process child_procs[ MAX_CHILDREN ]; -child_process *dead_child = NULL; static DWORD WINAPI reader_thread (void *arg); @@ -1042,9 +1041,6 @@ create_child (char *exe, char *cmdline, char *env, int is_gui_app, if (cp->pid < 0) cp->pid = -cp->pid; - /* pid must fit in a Lisp_Int */ - cp->pid = cp->pid & INTMASK; - *pPid = cp->pid; return TRUE; @@ -1120,55 +1116,110 @@ reap_subprocess (child_process *cp) delete_child (cp); } -/* Wait for any of our existing child processes to die - When it does, close its handle - Return the pid and fill in the status if non-NULL. */ +/* Wait for a child process specified by PID, or for any of our + existing child processes (if PID is nonpositive) to die. When it + does, close its handle. Return the pid of the process that died + and fill in STATUS if non-NULL. */ -int -sys_wait (int *status) +pid_t +waitpid (pid_t pid, int *status, int options) { DWORD active, retval; int nh; - int pid; child_process *cp, *cps[MAX_CHILDREN]; HANDLE wait_hnd[MAX_CHILDREN]; + DWORD timeout_ms; + int dont_wait = (options & WNOHANG) != 0; nh = 0; - if (dead_child != NULL) + /* According to Posix: + + PID = -1 means status is requested for any child process. + + PID > 0 means status is requested for a single child process + whose pid is PID. + + PID = 0 means status is requested for any child process whose + process group ID is equal to that of the calling process. But + since Windows has only a limited support for process groups (only + for console processes and only for the purposes of passing + Ctrl-BREAK signal to them), and since we have no documented way + of determining whether a given process belongs to our group, we + treat 0 as -1. + + PID < -1 means status is requested for any child process whose + process group ID is equal to the absolute value of PID. Again, + since we don't support process groups, we treat that as -1. */ + if (pid > 0) { - /* We want to wait for a specific child */ - wait_hnd[nh] = dead_child->procinfo.hProcess; - cps[nh] = dead_child; - if (!wait_hnd[nh]) emacs_abort (); - nh++; - active = 0; - goto get_result; + int our_child = 0; + + /* We are requested to wait for a specific child. */ + for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) + { + /* Some child_procs might be sockets; ignore them. Also + ignore subprocesses whose output is not yet completely + read. */ + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && cp->pid == pid) + { + our_child = 1; + break; + } + } + if (our_child) + { + if (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + else if (dont_wait) + { + /* PID specifies our subprocess, but its status is not + yet available. */ + return 0; + } + } + if (nh == 0) + { + /* No such child process, or nothing to wait for, so fail. */ + errno = ECHILD; + return -1; + } } else { for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) - /* some child_procs might be sockets; ignore them */ - if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess - && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) - { - wait_hnd[nh] = cp->procinfo.hProcess; - cps[nh] = cp; - nh++; - } + { + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + } + if (nh == 0) + { + /* Nothing to wait on, so fail. */ + errno = ECHILD; + return -1; + } } - if (nh == 0) - { - /* Nothing to wait on, so fail */ - errno = ECHILD; - return -1; - } + if (dont_wait) + timeout_ms = 0; + else + timeout_ms = 1000; /* check for quit about once a second. */ do { - /* Check for quit about once a second. */ QUIT; - active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000); + active = WaitForMultipleObjects (nh, wait_hnd, FALSE, timeout_ms); } while (active == WAIT_TIMEOUT); if (active == WAIT_FAILED) @@ -1198,8 +1249,10 @@ get_result: } if (retval == STILL_ACTIVE) { - /* Should never happen */ + /* Should never happen. */ DebPrint (("Wait.WaitForMultipleObjects returned an active process\n")); + if (pid > 0 && dont_wait) + return 0; errno = EINVAL; return -1; } @@ -1213,6 +1266,8 @@ get_result: else retval <<= 8; + if (pid > 0 && active != 0) + emacs_abort (); cp = cps[active]; pid = cp->pid; #ifdef FULL_DEBUG @@ -2001,9 +2056,7 @@ count_children: DebPrint (("select calling SIGCHLD handler for pid %d\n", cp->pid)); #endif - dead_child = cp; sig_handlers[SIGCHLD] (SIGCHLD); - dead_child = NULL; } } else if (fdindex[active] == -1) -- 2.39.2