diff options
author | Philipp Stephani <phst@google.com> | 2021-01-10 16:31:12 +0100 |
---|---|---|
committer | Philipp Stephani <phst@google.com> | 2021-01-16 19:46:44 +0100 |
commit | 8f0ce42d3eb9b212424a4a25a376287ffc94a73e (patch) | |
tree | b05a83f36c0617184079a9fa5ce33a945e7e736e /src | |
parent | 66756df286bea6efd3f9a8290e38e8d77bdf0264 (diff) | |
download | emacs-8f0ce42d3eb9b212424a4a25a376287ffc94a73e.tar.gz |
Fix deadlock when receiving SIGCHLD during 'pselect'.
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/process.c | 94 |
1 files changed, 93 insertions, 1 deletions
diff --git a/src/process.c b/src/process.c index dac7d0440fa..474c87089e0 100644 --- a/src/process.c +++ b/src/process.c @@ -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. |