From 5c00a0ee331ed77ee91a75ab33612afeb1a91088 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Sun, 18 Aug 2013 20:22:37 +0200
Subject: [PATCH] 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.
---
 include/uv-unix.h  |   1 +
 src/unix/process.c | 100 ++++++++++++++++++++++++---------------------
 2 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/include/uv-unix.h b/include/uv-unix.h
index d8b96a0a..965fbafb 100644
--- a/include/uv-unix.h
+++ b/include/uv-unix.h
@@ -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;                                                       \
diff --git a/src/unix/process.c b/src/unix/process.c
index cd369c7d..b7f6ac75 100644
--- a/src/unix/process.c
+++ b/src/unix/process.c
@@ -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));