unix: fix SIGCHLD waitpid() race in process.c
Before this commit, multiple event loops raced with each other when a SIGCHLD signal was received. More concretely, it was possible for event loop A to consume waitpid() events that should have been delivered to event loop B. This commit addresses that by doing a linear scan over the list of child processes. An O(n) scan is not terribly efficient but the actual performance impact is not measurable in a benchmark that spawns rounds of several thousands instances of /bin/false. For the time being, this patch will suffice; we can always revisit it later. Fixes #887.
This commit is contained in:
parent
556fe1a659
commit
5c00a0ee33
@ -288,6 +288,7 @@ typedef struct {
|
||||
#define UV_PROCESS_PRIVATE_FIELDS \
|
||||
void* queue[2]; \
|
||||
int errorno; \
|
||||
int status; \
|
||||
|
||||
#define UV_FS_PRIVATE_FIELDS \
|
||||
const char *new_path; \
|
||||
|
||||
@ -47,68 +47,73 @@ static QUEUE* uv__process_queue(uv_loop_t* loop, int pid) {
|
||||
}
|
||||
|
||||
|
||||
static uv_process_t* uv__process_find(uv_loop_t* loop, int pid) {
|
||||
uv_process_t* handle;
|
||||
static void uv__chld(uv_signal_t* handle, int signum) {
|
||||
uv_process_t* process;
|
||||
uv_loop_t* loop;
|
||||
int exit_status;
|
||||
int term_signal;
|
||||
unsigned int i;
|
||||
int status;
|
||||
pid_t pid;
|
||||
QUEUE pending;
|
||||
QUEUE* h;
|
||||
QUEUE* q;
|
||||
|
||||
h = uv__process_queue(loop, pid);
|
||||
|
||||
QUEUE_FOREACH(q, h) {
|
||||
handle = QUEUE_DATA(q, uv_process_t, queue);
|
||||
if (handle->pid == pid) return handle;
|
||||
}
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
static void uv__chld(uv_signal_t* handle, int signum) {
|
||||
uv_process_t* process;
|
||||
int exit_status;
|
||||
int term_signal;
|
||||
int status;
|
||||
pid_t pid;
|
||||
|
||||
assert(signum == SIGCHLD);
|
||||
|
||||
for (;;) {
|
||||
do
|
||||
pid = waitpid(-1, &status, WNOHANG);
|
||||
while (pid == -1 && errno == EINTR);
|
||||
QUEUE_INIT(&pending);
|
||||
loop = handle->loop;
|
||||
|
||||
if (pid == 0)
|
||||
return;
|
||||
for (i = 0; i < ARRAY_SIZE(loop->process_handles); i++) {
|
||||
h = loop->process_handles + i;
|
||||
q = QUEUE_HEAD(h);
|
||||
|
||||
if (pid == -1) {
|
||||
if (errno == ECHILD)
|
||||
return; /* XXX stop signal watcher? */
|
||||
else
|
||||
abort();
|
||||
while (q != h) {
|
||||
process = QUEUE_DATA(q, uv_process_t, queue);
|
||||
q = QUEUE_NEXT(q);
|
||||
|
||||
do
|
||||
pid = waitpid(process->pid, &status, WNOHANG);
|
||||
while (pid == -1 && errno == EINTR);
|
||||
|
||||
if (pid == 0)
|
||||
continue;
|
||||
|
||||
if (pid == -1) {
|
||||
if (errno != ECHILD)
|
||||
abort();
|
||||
continue;
|
||||
}
|
||||
|
||||
process->status = status;
|
||||
QUEUE_REMOVE(&process->queue);
|
||||
QUEUE_INSERT_TAIL(&pending, &process->queue);
|
||||
}
|
||||
|
||||
process = uv__process_find(handle->loop, pid);
|
||||
if (process == NULL)
|
||||
continue; /* XXX bug? abort? */
|
||||
while (!QUEUE_EMPTY(&pending)) {
|
||||
q = QUEUE_HEAD(&pending);
|
||||
QUEUE_REMOVE(q);
|
||||
QUEUE_INIT(q);
|
||||
|
||||
uv__handle_stop(process);
|
||||
process = QUEUE_DATA(q, uv_process_t, queue);
|
||||
uv__handle_stop(process);
|
||||
|
||||
if (process->exit_cb == NULL)
|
||||
continue;
|
||||
if (process->exit_cb == NULL)
|
||||
continue;
|
||||
|
||||
exit_status = 0;
|
||||
term_signal = 0;
|
||||
exit_status = 0;
|
||||
if (WIFEXITED(process->status))
|
||||
exit_status = WEXITSTATUS(process->status);
|
||||
|
||||
if (WIFEXITED(status))
|
||||
exit_status = WEXITSTATUS(status);
|
||||
term_signal = 0;
|
||||
if (WIFSIGNALED(process->status))
|
||||
term_signal = WTERMSIG(process->status);
|
||||
|
||||
if (WIFSIGNALED(status))
|
||||
term_signal = WTERMSIG(status);
|
||||
if (process->errorno != 0)
|
||||
exit_status = process->errorno; /* execve() failed */
|
||||
|
||||
if (process->errorno)
|
||||
exit_status = process->errorno; /* execve() failed */
|
||||
|
||||
process->exit_cb(process, exit_status, term_signal);
|
||||
process->exit_cb(process, exit_status, term_signal);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -428,6 +433,7 @@ int uv_spawn(uv_loop_t* loop,
|
||||
|
||||
uv__close(signal_pipe[1]);
|
||||
|
||||
process->status = 0;
|
||||
process->errorno = 0;
|
||||
do
|
||||
r = read(signal_pipe[0], &process->errorno, sizeof(process->errorno));
|
||||
|
||||
Loading…
Reference in New Issue
Block a user