[Mac] With sandbox early startup, start the sandbox after the port exchange

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
normal
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: haik, Assigned: haik)

Tracking

65 Branch
mozilla65
Unspecified
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 months ago
With the Mac sandbox early startup mode (security.sandbox.content.mac.earlyinit=true), the sandbox is started in the content process before the content process sends a message to the parent with its port descriptors. The parent process doesn't start child processes on the main thread, but it does block on the main thread in ContentParent::LaunchSubprocess() until the child has reached this point. As a result, by starting the sandbox before this time, on slower Macs, it resulted in ContentParent::LaunchSubprocess() being much slower because sandbox_init_with_parameters() is a very expensive call. Previously sandbox_init_with_parameters() was run later after receiving an async message from the parent. On a 2008 MacBook running OS X 10.9, I measured sandbox_init_with_parameters() to take over 500ms on average. On try hardware (machine t-yosemite-r7-186 running 10.10), I measured it taking on average 122ms during a Talos test with about 50 invocations (min: 73ms, max: 252ms, median: 120ms).

By starting the sandbox after the port exchange, we avoid ContentParent::LaunchSubprocess() taking extra time in the parent and still prevent access to the Dock and other services listed in bug 1431441. (On Mojave, I found that the Dock connection occurs in the [NSApplication sharedApplication] call from nsAppShell::Init()). And with this change, I couldn't reproduce any of the Talos regressions reported on bug 1431441.

Here are some rough measurements from other machines for the execution time of sandbox_init_with_parameters(). Note that the time is dependent on the policy string used.

  2008 MacBook with macOS 10.9:                         500ms
  try hardware (t-yosemite-r7-186) with macOS 10.10:    122ms
  2015 MacBook Air with macOS 10.14:                     15ms
  2012 MacBook Pro with macOS 10.11:                     14ms
  2018 MacBook Pro with macOS 10.14:                      8ms
(Assignee)

Updated

6 months ago
Assignee: nobody → haftandilian
Blocks: 1501126
Priority: -- → P1
(Assignee)

Comment 1

6 months ago
Don't start the sandbox until after the port exchange so the parent process does not have to wait longer in ContentParent::LaunchSubprocess() for the (expensive) sandbox_init_with_parameters call to complete in the child. Remove the policy rule allowing access to the parent port now that it is already open when the sandbox is initialized and therefore not needed.
(Assignee)

Updated

6 months ago
Blocks: 1505573

Comment 2

5 months ago
Pushed by haftandilian@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b246415f6864
[Mac] With sandbox early startup, start the sandbox after the port exchange r=Alex_Gaynor

Comment 3

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b246415f6864
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.