procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix deadlock when receiving SIGCHLD during 'pselect'.

Browse source 
If we receive and handle a SIGCHLD signal for a process while waiting
for that process, 'pselect' might never return.  Instead, we have to
explicitly 'pselect' that the process status has changed.  We do this
by writing to a pipe in the SIGCHLD handler and having
'wait_reading_process_output' select on it.

* src/process.c (child_signal_init): New helper function to create a
pipe for SIGCHLD notifications.
(child_signal_read, child_signal_notify): New helper functions to
read from/write to the child signal pipe.
(create_process): Initialize the child signal pipe on first use.
(handle_child_signal): Notify waiters that a process status has
changed.
(wait_reading_process_output): Make sure that we also catch
SIGCHLD/process status changes.

* test/src/process-tests.el
(process-tests/fd-setsize-no-crash/make-process): Remove workaround,
which is no longer needed.
This commit is contained in:
Philipp Stephani 2021-01-10 16:31:12 +01:00
parent 66756df286
commit 8f0ce42d3e
2 changed files with 93 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

94
src/process.c
View file

@ -283,6 +283,16 @@ static int max_desc;
the file descriptor of a socket that is already bound. */
static int external_sock_fd;
/* File descriptor that becomes readable when we receive SIGCHLD. */
static int child_signal_read_fd = -1;
/* The write end thereof. The SIGCHLD handler writes to this file
descriptor to notify `wait_reading_process_output' of process
status changes. */
static int child_signal_write_fd = -1;
static void child_signal_init (void);
static void child_signal_read (int, void *);
static void child_signal_notify (void);
/* Indexed by descriptor, gives the process (if any) for that descriptor. */
static Lisp_Object chan_process[FD_SETSIZE];
static void wait_for_socket_fds (Lisp_Object, char const *);
@ -2060,6 +2070,10 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
Lisp_Object lisp_pty_name = Qnil;
sigset_t oldset;
/* Ensure that the SIGCHLD handler can notify
`wait_reading_process_output'. */
child_signal_init ();
inchannel = outchannel = -1;
if (p->pty_flag)
@ -5395,6 +5409,14 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
check_write = true;
}
/* We have to be informed when we receive a SIGCHLD signal for
an asynchronous process. Otherwise this might deadlock if we
receive a SIGCHLD during `pselect'. */
int child_fd = child_signal_read_fd;
eassert (0 <= child_fd);
eassert (child_fd < FD_SETSIZE);
FD_SET (child_fd, &Available);
/* If frame size has changed or the window is newly mapped,
redisplay now, before we start to wait. There is a race
condition here; if a SIGIO arrives between now and the select
@ -7114,7 +7136,70 @@ process has been transmitted to the serial port. */)
subprocesses which the main thread should not reap. For example,
if the main thread attempted to reap an already-reaped child, it
might inadvertently reap a GTK-created process that happened to
have the same process ID. */
have the same process ID.
To avoid a deadlock when receiving SIGCHLD while
`wait_reading_process_output' is in `pselect', the SIGCHLD handler
will notify the `pselect' using a pipe. */
/* Set up `child_signal_read_fd' and `child_signal_write_fd'. */
static void
child_signal_init (void)
{
/* Either both are initialized, or both are uninitialized. */
eassert ((child_signal_read_fd < 0) == (child_signal_write_fd < 0));
if (0 <= child_signal_read_fd)
return; /* already done */
int fds[2];
if (emacs_pipe (fds) < 0)
report_file_error ("Creating pipe for child signal", Qnil);
if (FD_SETSIZE <= fds[0])
{
/* Since we need to `pselect' on the read end, it has to fit
into an `fd_set'. */
emacs_close (fds[0]);
emacs_close (fds[1]);
report_file_errno ("Creating pipe for child signal", Qnil,
EMFILE);
}
/* We leave the file descriptors open until the Emacs process
exits. */
eassert (0 <= fds[0]);
eassert (0 <= fds[1]);
add_read_fd (fds[0], child_signal_read, NULL);
fd_callback_info[fds[0]].flags &= ~KEYBOARD_FD;
child_signal_read_fd = fds[0];
child_signal_write_fd = fds[1];
}
/* Consume a process status change. */
static void
child_signal_read (int fd, void *data)
{
eassert (0 <= fd);
eassert (fd == child_signal_read_fd);
char dummy;
if (emacs_read (fd, &dummy, 1) < 0)
emacs_perror ("reading from child signal FD");
}
/* Notify `wait_reading_process_output' of a process status
change. */
static void
child_signal_notify (void)
{
int fd = child_signal_write_fd;
eassert (0 <= fd);
char dummy = 0;
if (emacs_write (fd, &dummy, 1) != 1)
emacs_perror ("writing to child signal FD");
}
/* LIB_CHILD_HANDLER is a SIGCHLD handler that Emacs calls while doing
its own SIGCHLD handling. On POSIXish systems, glib needs this to
@ -7152,6 +7237,7 @@ static void
handle_child_signal (int sig)
{
Lisp_Object tail, proc;
bool changed = false;
/* Find the process that signaled us, and record its status. */
@ -7174,6 +7260,7 @@ handle_child_signal (int sig)
eassert (ok);
if (child_status_changed (deleted_pid, 0, 0))
{
changed = true;
if (STRINGP (XCDR (head)))
unlink (SSDATA (XCDR (head)));
XSETCAR (tail, Qnil);
@ -7191,6 +7278,7 @@ handle_child_signal (int sig)
&& child_status_changed (p->pid, &status, WUNTRACED | WCONTINUED))
{
/* Change the status of the process that was found. */
changed = true;
p->tick = ++process_tick;
p->raw_status = status;
p->raw_status_new = 1;
@ -7210,6 +7298,10 @@ handle_child_signal (int sig)
}
}
if (changed)
/* Wake up `wait_reading_process_output'. */
child_signal_notify ();
lib_child_handler (sig);
#ifdef NS_IMPL_GNUSTEP
/* NSTask in GNUstep sets its child handler each time it is called.

5
test/src/process-tests.el
View file

@ -576,11 +576,6 @@ FD_SETSIZE file descriptors (Bug#24325)."
(should (memq (process-status process) '(run exit)))
(when (process-live-p process)
(process-send-eof process))
;; FIXME: This `sleep-for' shouldn't be needed. It
;; indicates a bug in Emacs; perhaps SIGCHLD is
;; received in parallel with `accept-process-output',
;; causing the latter to hang.
(sleep-for 0.1)
(while (accept-process-output process))
(should (eq (process-status process) 'exit))
;; If there's an error between fork and exec, Emacs