Bug 1370027: Part 1 - Cleanly handle a subprocess child being reaped by NSPR. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 06 Jun 2017 16:00:53 -0700
changeset 589837 49f1bf78060241e88d94593aa732ba5bb87411bc
parent 589785 78aa5a83314a9d35fe03c8f7ab552ccfa81e1190
child 589838 5c17ac1582290a7f06a306630cb4b80b3f015389
push id62541
push usermaglione.k@gmail.com
push dateTue, 06 Jun 2017 23:22:50 +0000
reviewersaswan
bugs1370027
milestone55.0a1
Bug 1370027: Part 1 - Cleanly handle a subprocess child being reaped by NSPR. r?aswan The first time any other code in the parent process uses NSPR (usually via nsIProcess) to spawn a new process, it spawns a thread to contuously wait for any child process to exit. This thread winds up reaping our child processes before we get the chance to wait for them, which leads us to continuously poll for them to exit. We don't have a good way to handle this, but checking the error status of waitpid at least prevents us from failing catastrophically. MozReview-Commit-ID: 75Z1yUHUmjy
dom/system/OSFileConstants.cpp
toolkit/modules/subprocess/subprocess_worker_unix.js
--- a/dom/system/OSFileConstants.cpp
+++ b/dom/system/OSFileConstants.cpp
@@ -542,16 +542,17 @@ static const dom::ConstantSpec gLibcProp
   // error values
   INT_CONSTANT(EACCES),
   INT_CONSTANT(EAGAIN),
   INT_CONSTANT(EBADF),
   INT_CONSTANT(EEXIST),
   INT_CONSTANT(EFAULT),
   INT_CONSTANT(EFBIG),
   INT_CONSTANT(EINVAL),
+  INT_CONSTANT(EINTR),
   INT_CONSTANT(EIO),
   INT_CONSTANT(EISDIR),
 #if defined(ELOOP) // not defined with VC9
   INT_CONSTANT(ELOOP),
 #endif // defined(ELOOP)
   INT_CONSTANT(EMFILE),
   INT_CONSTANT(ENAMETOOLONG),
   INT_CONSTANT(ENFILE),
--- a/toolkit/modules/subprocess/subprocess_worker_unix.js
+++ b/toolkit/modules/subprocess/subprocess_worker_unix.js
@@ -460,29 +460,35 @@ class Process extends BaseProcess {
   wait() {
     if (this.exitCode !== null) {
       return this.exitCode;
     }
 
     let status = ctypes.int();
 
     let res = libc.waitpid(this.pid, status.address(), LIBC.WNOHANG);
-    if (res == this.pid) {
-      let sig = unix.WTERMSIG(status.value);
-      if (sig) {
-        this.exitCode = -sig;
-      } else {
-        this.exitCode = unix.WEXITSTATUS(status.value);
-      }
+    // If there's a failure here and we get any errno other than EINTR, it
+    // means that the process has been reaped by another thread (most likely
+    // the nspr process wait thread), and its actual exit status is not
+    // available to us. In that case, we have to assume success.
+    if (res == 0 || (res == -1 && ctypes.errno == LIBC.EINTR)) {
+      return null;
+    }
 
-      this.fd.dispose();
-      io.updatePollFds();
-      this.resolveExit(this.exitCode);
-      return this.exitCode;
+    let sig = unix.WTERMSIG(status.value);
+    if (sig) {
+      this.exitCode = -sig;
+    } else {
+      this.exitCode = unix.WEXITSTATUS(status.value);
     }
+
+    this.fd.dispose();
+    io.updatePollFds();
+    this.resolveExit(this.exitCode);
+    return this.exitCode;
   }
 }
 
 io = {
   pollFds: null,
   pollHandlers: null,
 
   pipes: new Map(),