|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
This is the other half of bug 1400042: removing the inherently racy base::SetAllFDsToCloseOnExec from how we launch child processes on Mac. I suggested switching the other BSDs to fork/exec, but on OS X there's Mach weirdness we might run into (see https://crbug.com/179923), and there's also the cross-architecture NPAPI plugin support from bug 590057 and bug 592951 which I don't know if we still need. (Also, fork() is inherently expensive and scales linearly with the forking process's virtual memory size; see https://crbug.com/179923#c35. If posix_spawn can work, it's probably faster.) Fortunately, Apple has a nonstandard extension, POSIX_SPAWN_CLOEXEC_DEFAULT, which apparently does exactly what we want. https://crbug.com/179923#c31 says that “it 1) doesn't exist on 10.6 and 2) causes kernel panics on 10.7”, but according to https://support.mozilla.org/en-US/kb/firefox-osx our oldest supported version is 10.9. Chromium seems to use it by default now. So it might be enough to just throw in a posix_spawnattr_setflags and remove the SetAllFDsToCloseOnExec.
> and there's also the cross-architecture NPAPI plugin support from bug 590057 and bug 592951 which I don't know if we still need. We dropped support for 32-bit OS X, so we don't need that anymore.
I was already poking at process_util_mac.mm for bug 1401790, and it was easier to fix this than avoid fixing it.
Assignee: nobody → jld
Comment on attachment 8917596 [details] Bug 1400061 - Stop using SetAllFDsToCloseOnExec when launching processes on OS X. https://reviewboard.mozilla.org/r/188546/#review195698
Attachment #8917596 - Flags: review?(wmccloskey) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/7fc1873ffe07 Stop using SetAllFDsToCloseOnExec when launching processes on OS X. r=billm
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.