commit f7e0d580f4159357cfa3dc69cb9ed4d9027e9b9f Author: Kris Maglione maglione.k@gmail.com Date: Tue Jun 6 16:00:53 2017 -0700
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
--HG-- extra : rebase_source : db45f781190b6fc84873c32c611134326736a1ba
This closes our bug 25389. --- dom/system/OSFileConstants.cpp | 1 + .../modules/subprocess/subprocess_worker_unix.js | 28 +++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/dom/system/OSFileConstants.cpp b/dom/system/OSFileConstants.cpp index 945233f4c879..7d6aafff61cb 100644 --- a/dom/system/OSFileConstants.cpp +++ b/dom/system/OSFileConstants.cpp @@ -549,6 +549,7 @@ static const dom::ConstantSpec gLibcProperties[] = 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 diff --git a/toolkit/modules/subprocess/subprocess_worker_unix.js b/toolkit/modules/subprocess/subprocess_worker_unix.js index 839402deb189..1584a9df508b 100644 --- a/toolkit/modules/subprocess/subprocess_worker_unix.js +++ b/toolkit/modules/subprocess/subprocess_worker_unix.js @@ -463,19 +463,25 @@ class Process extends BaseProcess { 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; } }