1
0
mirror of https://git.savannah.gnu.org/git/emacs.git synced 2025-02-03 20:24:29 +00:00

Revert recent change for Bug#8855.

As reported by Harald Hanche-Olsen in
<http://lists.gnu.org/archive/html/emacs-devel/2012-11/msg00445.html>
the change introduces a further bug, of creating lots of zombie
processes in some cases.  Further work is needed to come up with a
better fix for Bug#8855.
This commit is contained in:
Paul Eggert 2012-11-24 00:24:11 -08:00
parent 0c5ef13335
commit d454751173
9 changed files with 166 additions and 221 deletions

View File

@ -1,3 +1,7 @@
2012-11-24 Paul Eggert <eggert@cs.ucla.edu>
Revert recent change for Bug#8855; see ../src/ChangeLog.
2012-11-23 Eli Zaretskii <eliz@gnu.org>
Fix a race condition with glib (Bug#8855).

View File

@ -963,7 +963,7 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#undef HAVE_SYS_VLIMIT_H
/* Define to 1 if you have <sys/wait.h> that is POSIX.1 compatible. */
#define HAVE_SYS_WAIT_H 1
#undef HAVE_SYS_WAIT_H
/* Define to 1 if you have the <term.h> header file. */
#undef HAVE_TERM_H

View File

@ -185,12 +185,15 @@ 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

View File

@ -1,33 +0,0 @@
/* 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 <http://www.gnu.org/licenses/>. */
#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_ */

View File

@ -1,3 +1,12 @@
2012-11-24 Paul Eggert <eggert@cs.ucla.edu>
Revert recent change for Bug#8855.
As reported by Harald Hanche-Olsen in
<http://lists.gnu.org/archive/html/emacs-devel/2012-11/msg00445.html>
the change introduces a further bug, of creating lots of zombie
processes in some cases. Further work is needed to come up with a
better fix for Bug#8855.
2012-11-24 Eli Zaretskii <eliz@gnu.org>
* xdisp.c (draw_glyphs): Don't draw in mouse face if mouse

View File

@ -130,6 +130,14 @@ extern int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *,
EMACS_TIME *, void *);
#endif
#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
<http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>.
These lines can be removed once the GCC bug is fixed. */
@ -787,8 +795,9 @@ get_process (register Lisp_Object name)
#ifdef SIGCHLD
/* Fdelete_process promises to immediately forget about the process, but in
reality, Emacs needs to remember those processes until they have been
treated by the SIGCHLD handler and waitpid has been invoked on them;
otherwise they might fill up the kernel's process table. */
treated by the SIGCHLD handler; otherwise this handler would consider the
process as being synchronous and say that the synchronous process is
dead. */
static Lisp_Object deleted_pid_list;
#endif
@ -1695,7 +1704,16 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
if (inchannel > max_process_desc)
max_process_desc = inchannel;
/* This may signal an error. */
/* Until we store the proper pid, enable the SIGCHLD handler
to recognize an unknown pid as standing for this process.
It is very important not to let this `marker' value stay
in the table after this function has returned; if it does
it might cause call-process to hang and subsequent asynchronous
processes to get their return values scrambled. */
XPROCESS (process)->pid = -1;
/* This must be called after the above line because it may signal an
error. */
setup_process_coding_systems (process);
encoded_current_dir = ENCODE_FILE (current_dir);
@ -1862,8 +1880,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
#endif
XPROCESS (process)->pid = pid;
if (0 <= pid)
XPROCESS (process)->alive = 1;
/* Stop blocking signals in the parent. */
#ifdef SIGCHLD
@ -6257,35 +6273,9 @@ process has been transmitted to the serial port. */)
return process;
}
/* If the status of the process DESIRED has changed, return true and
set *STATUS to its exit status; otherwise, return false.
If HAVE is nonnegative, assume that HAVE = waitpid (HAVE, STATUS, ...)
has already been invoked, and do not invoke waitpid again. */
static bool
process_status_retrieved (pid_t desired, pid_t have, int *status)
{
if (have < 0)
{
/* Invoke waitpid only with a known process ID; do not invoke
waitpid with a nonpositive argument. Otherwise, Emacs might
reap an unwanted process by mistake. For example, invoking
waitpid (-1, ...) can mess up glib by reaping glib's subprocesses,
so that another thread running glib won't find them. */
do
have = waitpid (desired, status, WNOHANG | WUNTRACED);
while (have < 0 && errno == EINTR);
}
return have == desired;
}
/* If PID is nonnegative, the child process PID with wait status W has
changed its status; record this and return true.
If PID is negative, ignore W, and look for known child processes
of Emacs whose status have changed. For each one found, record its new
status.
/* On receipt of a signal that a child status has changed, loop asking
about children with changed statuses until the system says there
are no more.
All we do is change the status; we do not run sentinels or print
notifications. That is saved for the next time keyboard input is
@ -6308,15 +6298,13 @@ process_status_retrieved (pid_t desired, pid_t have, int *status)
** Malloc WARNING: This should never call malloc either directly or
indirectly; if it does, that is a bug */
/* Record the changed status of the child process PID with wait status W. */
void
record_child_status_change (pid_t pid, int w)
{
#ifdef SIGCHLD
/* 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;
Lisp_Object proc;
struct Lisp_Process *p;
Lisp_Object tail;
/* Find the process that signaled us, and record its status. */
@ -6324,69 +6312,68 @@ record_child_status_change (pid_t pid, int w)
/* The process can have been deleted by Fdelete_process. */
for (tail = deleted_pid_list; CONSP (tail); tail = XCDR (tail))
{
bool all_pids_are_fixnums
= (MOST_NEGATIVE_FIXNUM <= TYPE_MINIMUM (pid_t)
&& TYPE_MAXIMUM (pid_t) <= MOST_POSITIVE_FIXNUM);
Lisp_Object xpid = XCAR (tail);
if (all_pids_are_fixnums ? INTEGERP (xpid) : NUMBERP (xpid))
if ((INTEGERP (xpid) && pid == XINT (xpid))
|| (FLOATP (xpid) && pid == XFLOAT_DATA (xpid)))
{
pid_t deleted_pid;
if (INTEGERP (xpid))
deleted_pid = XINT (xpid);
else
deleted_pid = XFLOAT_DATA (xpid);
if (process_status_retrieved (deleted_pid, pid, &w))
{
XSETCAR (tail, Qnil);
if (record_at_most_one_child)
return;
}
XSETCAR (tail, Qnil);
return;
}
}
/* Otherwise, if it is asynchronous, it is in Vprocess_alist. */
p = 0;
for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
{
Lisp_Object proc = XCDR (XCAR (tail));
struct Lisp_Process *p = XPROCESS (proc);
if (p->alive && process_status_retrieved (p->pid, pid, &w))
{
/* Change the status of the process that was found. */
p->tick = ++process_tick;
p->raw_status = w;
p->raw_status_new = 1;
/* If process has terminated, stop waiting for its output. */
if (WIFSIGNALED (w) || WIFEXITED (w))
{
int clear_desc_flag = 0;
p->alive = 0;
if (p->infd >= 0)
clear_desc_flag = 1;
/* clear_desc_flag avoids a compiler bug in Microsoft C. */
if (clear_desc_flag)
{
FD_CLR (p->infd, &input_wait_mask);
FD_CLR (p->infd, &non_keyboard_wait_mask);
}
}
/* Tell wait_reading_process_output that it needs to wake up and
look around. */
if (input_available_clear_time)
*input_available_clear_time = make_emacs_time (0, 0);
if (record_at_most_one_child)
return;
}
proc = XCDR (XCAR (tail));
p = XPROCESS (proc);
if (EQ (p->type, Qreal) && p->pid == pid)
break;
p = 0;
}
if (0 <= pid)
{
/* The caller successfully waited for a pid but no asynchronous
process was found for it, so this is a synchronous process. */
/* Look for an asynchronous process whose pid hasn't been filled
in yet. */
if (! p)
for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
{
proc = XCDR (XCAR (tail));
p = XPROCESS (proc);
if (p->pid == -1)
break;
p = 0;
}
/* Change the status of the process that was found. */
if (p)
{
int clear_desc_flag = 0;
p->tick = ++process_tick;
p->raw_status = w;
p->raw_status_new = 1;
/* If process has terminated, stop waiting for its output. */
if ((WIFSIGNALED (w) || WIFEXITED (w))
&& p->infd >= 0)
clear_desc_flag = 1;
/* We use clear_desc_flag to avoid a compiler bug in Microsoft C. */
if (clear_desc_flag)
{
FD_CLR (p->infd, &input_wait_mask);
FD_CLR (p->infd, &non_keyboard_wait_mask);
}
/* Tell wait_reading_process_output that it needs to wake up and
look around. */
if (input_available_clear_time)
*input_available_clear_time = make_emacs_time (0, 0);
}
/* There was no asynchronous process found for that pid: we have
a synchronous process. */
else
{
synch_process_alive = 0;
/* Report the status of the synchronous process. */
@ -6405,10 +6392,38 @@ record_child_status_change (pid_t pid, int w)
#ifdef SIGCHLD
/* On some systems, the SIGCHLD handler must return right away. If
any more processes want to signal us, we will get another signal.
Otherwise, loop around to use up all the processes that have
something to tell us. */
#if (defined WINDOWSNT \
|| (defined USG && !defined GNU_LINUX \
&& !(defined HPUX && defined WNOHANG)))
enum { CAN_HANDLE_MULTIPLE_CHILDREN = 0 };
#else
enum { CAN_HANDLE_MULTIPLE_CHILDREN = 1 };
#endif
static void
handle_child_signal (int sig)
{
record_child_status_change (-1, 0);
do
{
pid_t pid;
int status;
do
pid = waitpid (-1, &status, WNOHANG | WUNTRACED);
while (pid < 0 && errno == EINTR);
/* PID == 0 means no processes found, PID == -1 means a real failure.
Either way, we have done all our job. */
if (pid <= 0)
break;
record_child_status_change (pid, status);
}
while (CAN_HANDLE_MULTIPLE_CHILDREN);
}
static void

View File

@ -142,9 +142,6 @@ struct Lisp_Process
/* Flag to set coding-system of the process buffer from the
coding_system used to decode process output. */
unsigned int inherit_coding_system_flag : 1;
/* Whether the process is alive, i.e., can be waited for. Running
processes can be waited for, but exited and fake processes cannot. */
unsigned int alive : 1;
/* Record the process status in the raw form in which it comes from `wait'.
This is to avoid consing in a signal handler. The `raw_status_new'
flag indicates that `raw_status' contains a new status that still

View File

@ -289,6 +289,10 @@ 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)
@ -302,8 +306,7 @@ wait_for_termination_1 (pid_t pid, int interruptible)
break;
}
/* Note: the MS-Windows emulation of waitpid calls QUIT
internally. */
#endif /* not WINDOWSNT */
if (interruptible)
QUIT;
}

View File

@ -783,6 +783,7 @@ 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);
@ -1035,6 +1036,9 @@ 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;
@ -1110,110 +1114,55 @@ reap_subprocess (child_process *cp)
delete_child (cp);
}
/* 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. */
/* 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. */
pid_t
waitpid (pid_t pid, int *status, int options)
int
sys_wait (int *status)
{
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;
/* 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)
if (dead_child != NULL)
{
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;
}
/* 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;
}
else
{
for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--)
{
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;
}
/* 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 (dont_wait)
timeout_ms = 0;
else
timeout_ms = 1000; /* check for quit about once a second. */
if (nh == 0)
{
/* Nothing to wait on, so fail */
errno = ECHILD;
return -1;
}
do
{
/* Check for quit about once a second. */
QUIT;
active = WaitForMultipleObjects (nh, wait_hnd, FALSE, timeout_ms);
active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000);
} while (active == WAIT_TIMEOUT);
if (active == WAIT_FAILED)
@ -1243,10 +1192,8 @@ 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;
}
@ -1260,8 +1207,6 @@ get_result:
else
retval <<= 8;
if (pid > 0 && active != 0)
emacs_abort ();
cp = cps[active];
pid = cp->pid;
#ifdef FULL_DEBUG
@ -2050,7 +1995,9 @@ 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)