diff options
| author | Eli Zaretskii | 2012-11-17 18:46:45 +0200 |
|---|---|---|
| committer | Eli Zaretskii | 2012-11-17 18:46:45 +0200 |
| commit | 22bae83fa8c432780fe20202a660aa8c84f3087a (patch) | |
| tree | e58043c1a42681cc2ba63a20c5d85c84a1dbb6ad /src | |
| parent | a631d0e04747884855aa460cb903d1fd2ff106f4 (diff) | |
| download | emacs-22bae83fa8c432780fe20202a660aa8c84f3087a.tar.gz emacs-22bae83fa8c432780fe20202a660aa8c84f3087a.zip | |
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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/ChangeLog | 20 | ||||
| -rw-r--r-- | src/process.c | 20 | ||||
| -rw-r--r-- | src/sysdep.c | 7 | ||||
| -rw-r--r-- | src/w32proc.c | 127 |
4 files changed, 112 insertions, 62 deletions
diff --git a/src/ChangeLog b/src/ChangeLog index 1194fe099fa..45d48ba41cc 100644 --- a/src/ChangeLog +++ b/src/ChangeLog | |||
| @@ -1,3 +1,23 @@ | |||
| 1 | 2012-11-17 Eli Zaretskii <eliz@gnu.org> | ||
| 2 | |||
| 3 | * w32proc.c (create_child): Don't clip the PID of the child | ||
| 4 | process to fit into an Emacs integer, as this is no longer a | ||
| 5 | restriction. | ||
| 6 | (waitpid): Rename from sys_wait. Emulate a Posix 'waitpid' by | ||
| 7 | reaping only the process specified by PID argument, if that is | ||
| 8 | positive. Use PID instead of dead_child to know which process to | ||
| 9 | reap. Wait for the child to die only if WNOHANG is not in | ||
| 10 | OPTIONS. | ||
| 11 | (sys_select): Don't set dead_child. | ||
| 12 | |||
| 13 | * sysdep.c (wait_for_termination_1): Remove the WINDOWSNT portion, | ||
| 14 | as it is no longer needed. | ||
| 15 | |||
| 16 | * process.c (waitpid, WUNTRACED) [!WNOHANG]: Remove definitions, | ||
| 17 | no longer needed. | ||
| 18 | (record_child_status_change): Remove the setting of | ||
| 19 | record_at_most_one_child for the !WNOHANG case. | ||
| 20 | |||
| 1 | 2012-11-17 Paul Eggert <eggert@cs.ucla.edu> | 21 | 2012-11-17 Paul Eggert <eggert@cs.ucla.edu> |
| 2 | 22 | ||
| 3 | Fix problems in ns port found by static checking. | 23 | 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 *, | |||
| 130 | EMACS_TIME *, void *); | 130 | EMACS_TIME *, void *); |
| 131 | #endif | 131 | #endif |
| 132 | 132 | ||
| 133 | /* This is for DOS_NT ports. FIXME: Remove this old portability cruft | ||
| 134 | by having DOS_NT ports implement waitpid instead of wait. Nowadays | ||
| 135 | POSIXish hosts all define waitpid, WNOHANG, and WUNTRACED, as these | ||
| 136 | have been standard since POSIX.1-1988. */ | ||
| 137 | #ifndef WNOHANG | ||
| 138 | # undef waitpid | ||
| 139 | # define waitpid(pid, status, options) wait (status) | ||
| 140 | #endif | ||
| 141 | #ifndef WUNTRACED | ||
| 142 | # define WUNTRACED 0 | ||
| 143 | #endif | ||
| 144 | |||
| 145 | /* Work around GCC 4.7.0 bug with strict overflow checking; see | 133 | /* Work around GCC 4.7.0 bug with strict overflow checking; see |
| 146 | <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>. | 134 | <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>. |
| 147 | These lines can be removed once the GCC bug is fixed. */ | 135 | These lines can be removed once the GCC bug is fixed. */ |
| @@ -6295,17 +6283,9 @@ record_child_status_change (pid_t pid, int w) | |||
| 6295 | { | 6283 | { |
| 6296 | #ifdef SIGCHLD | 6284 | #ifdef SIGCHLD |
| 6297 | 6285 | ||
| 6298 | # ifdef WNOHANG | ||
| 6299 | /* On POSIXish hosts, record at most one child only if we already | 6286 | /* On POSIXish hosts, record at most one child only if we already |
| 6300 | know one child that has exited. */ | 6287 | know one child that has exited. */ |
| 6301 | bool record_at_most_one_child = 0 <= pid; | 6288 | bool record_at_most_one_child = 0 <= pid; |
| 6302 | # else | ||
| 6303 | /* On DOS_NT (the only porting target that lacks WNOHANG), | ||
| 6304 | record the status of at most one child process, since the SIGCHLD | ||
| 6305 | handler must return right away. If any more processes want to | ||
| 6306 | signal us, we will get another signal. */ | ||
| 6307 | bool record_at_most_one_child = 1; | ||
| 6308 | # endif | ||
| 6309 | 6289 | ||
| 6310 | Lisp_Object tail; | 6290 | Lisp_Object tail; |
| 6311 | 6291 | ||
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) | |||
| 289 | { | 289 | { |
| 290 | while (1) | 290 | while (1) |
| 291 | { | 291 | { |
| 292 | #ifdef WINDOWSNT | ||
| 293 | wait (0); | ||
| 294 | break; | ||
| 295 | #else /* not WINDOWSNT */ | ||
| 296 | int status; | 292 | int status; |
| 297 | int wait_result = waitpid (pid, &status, 0); | 293 | int wait_result = waitpid (pid, &status, 0); |
| 298 | if (wait_result < 0) | 294 | if (wait_result < 0) |
| @@ -306,7 +302,8 @@ wait_for_termination_1 (pid_t pid, int interruptible) | |||
| 306 | break; | 302 | break; |
| 307 | } | 303 | } |
| 308 | 304 | ||
| 309 | #endif /* not WINDOWSNT */ | 305 | /* Note: the MS-Windows emulation of waitpid calls QUIT |
| 306 | internally. */ | ||
| 310 | if (interruptible) | 307 | if (interruptible) |
| 311 | QUIT; | 308 | QUIT; |
| 312 | } | 309 | } |
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) | |||
| 789 | /* Child process management list. */ | 789 | /* Child process management list. */ |
| 790 | int child_proc_count = 0; | 790 | int child_proc_count = 0; |
| 791 | child_process child_procs[ MAX_CHILDREN ]; | 791 | child_process child_procs[ MAX_CHILDREN ]; |
| 792 | child_process *dead_child = NULL; | ||
| 793 | 792 | ||
| 794 | static DWORD WINAPI reader_thread (void *arg); | 793 | static DWORD WINAPI reader_thread (void *arg); |
| 795 | 794 | ||
| @@ -1042,9 +1041,6 @@ create_child (char *exe, char *cmdline, char *env, int is_gui_app, | |||
| 1042 | if (cp->pid < 0) | 1041 | if (cp->pid < 0) |
| 1043 | cp->pid = -cp->pid; | 1042 | cp->pid = -cp->pid; |
| 1044 | 1043 | ||
| 1045 | /* pid must fit in a Lisp_Int */ | ||
| 1046 | cp->pid = cp->pid & INTMASK; | ||
| 1047 | |||
| 1048 | *pPid = cp->pid; | 1044 | *pPid = cp->pid; |
| 1049 | 1045 | ||
| 1050 | return TRUE; | 1046 | return TRUE; |
| @@ -1120,55 +1116,110 @@ reap_subprocess (child_process *cp) | |||
| 1120 | delete_child (cp); | 1116 | delete_child (cp); |
| 1121 | } | 1117 | } |
| 1122 | 1118 | ||
| 1123 | /* Wait for any of our existing child processes to die | 1119 | /* Wait for a child process specified by PID, or for any of our |
| 1124 | When it does, close its handle | 1120 | existing child processes (if PID is nonpositive) to die. When it |
| 1125 | Return the pid and fill in the status if non-NULL. */ | 1121 | does, close its handle. Return the pid of the process that died |
| 1122 | and fill in STATUS if non-NULL. */ | ||
| 1126 | 1123 | ||
| 1127 | int | 1124 | pid_t |
| 1128 | sys_wait (int *status) | 1125 | waitpid (pid_t pid, int *status, int options) |
| 1129 | { | 1126 | { |
| 1130 | DWORD active, retval; | 1127 | DWORD active, retval; |
| 1131 | int nh; | 1128 | int nh; |
| 1132 | int pid; | ||
| 1133 | child_process *cp, *cps[MAX_CHILDREN]; | 1129 | child_process *cp, *cps[MAX_CHILDREN]; |
| 1134 | HANDLE wait_hnd[MAX_CHILDREN]; | 1130 | HANDLE wait_hnd[MAX_CHILDREN]; |
| 1131 | DWORD timeout_ms; | ||
| 1132 | int dont_wait = (options & WNOHANG) != 0; | ||
| 1135 | 1133 | ||
| 1136 | nh = 0; | 1134 | nh = 0; |
| 1137 | if (dead_child != NULL) | 1135 | /* According to Posix: |
| 1136 | |||
| 1137 | PID = -1 means status is requested for any child process. | ||
| 1138 | |||
| 1139 | PID > 0 means status is requested for a single child process | ||
| 1140 | whose pid is PID. | ||
| 1141 | |||
| 1142 | PID = 0 means status is requested for any child process whose | ||
| 1143 | process group ID is equal to that of the calling process. But | ||
| 1144 | since Windows has only a limited support for process groups (only | ||
| 1145 | for console processes and only for the purposes of passing | ||
| 1146 | Ctrl-BREAK signal to them), and since we have no documented way | ||
| 1147 | of determining whether a given process belongs to our group, we | ||
| 1148 | treat 0 as -1. | ||
| 1149 | |||
| 1150 | PID < -1 means status is requested for any child process whose | ||
| 1151 | process group ID is equal to the absolute value of PID. Again, | ||
| 1152 | since we don't support process groups, we treat that as -1. */ | ||
| 1153 | if (pid > 0) | ||
| 1138 | { | 1154 | { |
| 1139 | /* We want to wait for a specific child */ | 1155 | int our_child = 0; |
| 1140 | wait_hnd[nh] = dead_child->procinfo.hProcess; | 1156 | |
| 1141 | cps[nh] = dead_child; | 1157 | /* We are requested to wait for a specific child. */ |
| 1142 | if (!wait_hnd[nh]) emacs_abort (); | 1158 | for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) |
| 1143 | nh++; | 1159 | { |
| 1144 | active = 0; | 1160 | /* Some child_procs might be sockets; ignore them. Also |
| 1145 | goto get_result; | 1161 | ignore subprocesses whose output is not yet completely |
| 1162 | read. */ | ||
| 1163 | if (CHILD_ACTIVE (cp) | ||
| 1164 | && cp->procinfo.hProcess | ||
| 1165 | && cp->pid == pid) | ||
| 1166 | { | ||
| 1167 | our_child = 1; | ||
| 1168 | break; | ||
| 1169 | } | ||
| 1170 | } | ||
| 1171 | if (our_child) | ||
| 1172 | { | ||
| 1173 | if (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0) | ||
| 1174 | { | ||
| 1175 | wait_hnd[nh] = cp->procinfo.hProcess; | ||
| 1176 | cps[nh] = cp; | ||
| 1177 | nh++; | ||
| 1178 | } | ||
| 1179 | else if (dont_wait) | ||
| 1180 | { | ||
| 1181 | /* PID specifies our subprocess, but its status is not | ||
| 1182 | yet available. */ | ||
| 1183 | return 0; | ||
| 1184 | } | ||
| 1185 | } | ||
| 1186 | if (nh == 0) | ||
| 1187 | { | ||
| 1188 | /* No such child process, or nothing to wait for, so fail. */ | ||
| 1189 | errno = ECHILD; | ||
| 1190 | return -1; | ||
| 1191 | } | ||
| 1146 | } | 1192 | } |
| 1147 | else | 1193 | else |
| 1148 | { | 1194 | { |
| 1149 | for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) | 1195 | for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) |
| 1150 | /* some child_procs might be sockets; ignore them */ | 1196 | { |
| 1151 | if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess | 1197 | if (CHILD_ACTIVE (cp) |
| 1152 | && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) | 1198 | && cp->procinfo.hProcess |
| 1153 | { | 1199 | && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) |
| 1154 | wait_hnd[nh] = cp->procinfo.hProcess; | 1200 | { |
| 1155 | cps[nh] = cp; | 1201 | wait_hnd[nh] = cp->procinfo.hProcess; |
| 1156 | nh++; | 1202 | cps[nh] = cp; |
| 1157 | } | 1203 | nh++; |
| 1204 | } | ||
| 1205 | } | ||
| 1206 | if (nh == 0) | ||
| 1207 | { | ||
| 1208 | /* Nothing to wait on, so fail. */ | ||
| 1209 | errno = ECHILD; | ||
| 1210 | return -1; | ||
| 1211 | } | ||
| 1158 | } | 1212 | } |
| 1159 | 1213 | ||
| 1160 | if (nh == 0) | 1214 | if (dont_wait) |
| 1161 | { | 1215 | timeout_ms = 0; |
| 1162 | /* Nothing to wait on, so fail */ | 1216 | else |
| 1163 | errno = ECHILD; | 1217 | timeout_ms = 1000; /* check for quit about once a second. */ |
| 1164 | return -1; | ||
| 1165 | } | ||
| 1166 | 1218 | ||
| 1167 | do | 1219 | do |
| 1168 | { | 1220 | { |
| 1169 | /* Check for quit about once a second. */ | ||
| 1170 | QUIT; | 1221 | QUIT; |
| 1171 | active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000); | 1222 | active = WaitForMultipleObjects (nh, wait_hnd, FALSE, timeout_ms); |
| 1172 | } while (active == WAIT_TIMEOUT); | 1223 | } while (active == WAIT_TIMEOUT); |
| 1173 | 1224 | ||
| 1174 | if (active == WAIT_FAILED) | 1225 | if (active == WAIT_FAILED) |
| @@ -1198,8 +1249,10 @@ get_result: | |||
| 1198 | } | 1249 | } |
| 1199 | if (retval == STILL_ACTIVE) | 1250 | if (retval == STILL_ACTIVE) |
| 1200 | { | 1251 | { |
| 1201 | /* Should never happen */ | 1252 | /* Should never happen. */ |
| 1202 | DebPrint (("Wait.WaitForMultipleObjects returned an active process\n")); | 1253 | DebPrint (("Wait.WaitForMultipleObjects returned an active process\n")); |
| 1254 | if (pid > 0 && dont_wait) | ||
| 1255 | return 0; | ||
| 1203 | errno = EINVAL; | 1256 | errno = EINVAL; |
| 1204 | return -1; | 1257 | return -1; |
| 1205 | } | 1258 | } |
| @@ -1213,6 +1266,8 @@ get_result: | |||
| 1213 | else | 1266 | else |
| 1214 | retval <<= 8; | 1267 | retval <<= 8; |
| 1215 | 1268 | ||
| 1269 | if (pid > 0 && active != 0) | ||
| 1270 | emacs_abort (); | ||
| 1216 | cp = cps[active]; | 1271 | cp = cps[active]; |
| 1217 | pid = cp->pid; | 1272 | pid = cp->pid; |
| 1218 | #ifdef FULL_DEBUG | 1273 | #ifdef FULL_DEBUG |
| @@ -2001,9 +2056,7 @@ count_children: | |||
| 2001 | DebPrint (("select calling SIGCHLD handler for pid %d\n", | 2056 | DebPrint (("select calling SIGCHLD handler for pid %d\n", |
| 2002 | cp->pid)); | 2057 | cp->pid)); |
| 2003 | #endif | 2058 | #endif |
| 2004 | dead_child = cp; | ||
| 2005 | sig_handlers[SIGCHLD] (SIGCHLD); | 2059 | sig_handlers[SIGCHLD] (SIGCHLD); |
| 2006 | dead_child = NULL; | ||
| 2007 | } | 2060 | } |
| 2008 | } | 2061 | } |
| 2009 | else if (fdindex[active] == -1) | 2062 | else if (fdindex[active] == -1) |