Remove use of base::SetAllFDsToCloseOnExec from process_util_mac.mm

RESOLVED FIXED in Firefox 58

Status

()

Core
IPC
P3
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: jld, Assigned: jld)

Tracking

(Blocks: 1 bug)

Trunk
mozilla58
Points:
---

Firefox Tracking Flags

(firefox57 wontfix, firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
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.

Updated

5 months ago
Priority: -- → P3
(Assignee)

Comment 2

5 months ago
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 hidden (mozreview-request)
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+

Comment 5

4 months ago
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fc1873ffe07
Stop using SetAllFDsToCloseOnExec when launching processes on OS X. r=billm
https://hg.mozilla.org/mozilla-central/rev/7fc1873ffe07
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
status-firefox57: affected → wontfix
You need to log in before you can comment on or make changes to this bug.