Opened 19 months ago

Closed 18 months ago

Last modified 9 months ago

#21876 closed defect (fixed)

e10s is not enabled on Linux (and probably OS X) by default in ESR 52 based nightlies

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: ff52-esr, tbb-7.0-must-alpha, TorBrowserTeam201704R, tbb-no-uplift
Cc: mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

For some reason e10s is not enabled on Linux (and probably OS X) in ESR 52 based nightlies. Both our extensions and HTTPS-Everywhere + NoScript set multiprocessCompatible to true, so I wonder what is going on.

Child Tickets

Change History (26)

comment:1 Changed 19 months ago by gk

mcs/brade can you look at that one? Maybe that e10s extension is messing things up here? See comment:1:ticket:21875 for our nightly situation.

comment:2 Changed 19 months ago by cypherpunks

What do you see in " Multiprocess Windows" row on about:support?

On Win XP SP3 it is "0/1 (Disabled)" and
Extensions
Name Version Enabled ID
Application Update Service Helper 2.0 true aushelper@…
HTTPS Everywhere 5.2.7 true https-everywhere-eff@…
Multi-process staged rollout 1.9 true e10srollout@…
NoScript 5.0.2 true {73a6fe31-595d-460b-a920-fcc0f8843232}
Pocket 1.0.5 true firefox@…
Torbutton 1.9.7.1 true torbutton@…
TorLauncher 0.2.11.1 true tor-launcher@…
Web Compat 1.0 true webcompat@…

comment:3 Changed 19 months ago by cypherpunks

e10s is not enabled on Linux (and probably OS X) by default in ESR 52 based nightlies

It's not enabled by default anywhere (in FF too), because this feature is not ready for GA. So, it is rolling out to separate groups of users, and e10srollout add-on decides when and for whom to enable e10s.

comment:4 in reply to:  3 Changed 19 months ago by mcs

Replying to cypherpunks:

It's not enabled by default anywhere (in FF too), because this feature is not ready for GA. So, it is rolling out to separate groups of users, and e10srollout add-on decides when and for whom to enable e10s.

That is correct, although for all esr or release channel Firefox users, e10s does get enabled by the e10srollout add-on unless the user has an incompatible extension installed.

For our TB nightly builds the update channel is "default", which means that the e10srollout add-on does not do anything (the channel is unknown). See:

https://dxr.mozilla.org/mozilla-esr52/source/browser/extensions/e10srollout/bootstrap.js#61

The other part of the story is that for Firefox nightly and alpha builds, e10s is enabled by default but it is not for our nightly builds. Why not? Because RELEASE_OR_BETA is defined for our builds, which means the browser.tabs.remote.autostart.2 pref is not set to true. See:

https://dxr.mozilla.org/mozilla-esr52/source/browser/app/profile/firefox.js#1438

One solution is to patch browser/app/profile/firefox.js to remove the #ifndef RELEASE_OR_BETA. I don't think we want to arrange for RELEASE_OR_BETA to be defined because that may have other undesirable effects.

comment:5 Changed 19 months ago by mcs

Or, if we are going to keep using the e10srollout add-on, we should backport the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=1348576.

comment:6 Changed 19 months ago by cypherpunks

Also langpacks have "multiprocessCompatible":false,"runInSafeMode":false

comment:7 in reply to:  5 ; Changed 19 months ago by gk

Replying to mcs:

Or, if we are going to keep using the e10srollout add-on, we should backport the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=1348576.

I think keeping that extension, auditing it, and making sure it does not get some out-of-bound Mozilla update is the way to go. When backporting that fix we should, however, make sure the esr policy applies to us.

comment:8 in reply to:  7 Changed 19 months ago by cypherpunks

Replying to gk:

Replying to mcs:

Or, if we are going to keep using the e10srollout add-on, we should backport the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=1348576.

I think keeping that extension,

Sooner or later mandatory system add-on would appear.

auditing it,

Very good. (Treat it as a part of the browser.)

and making sure it does not get some out-of-bound Mozilla update is the way to go.

Why don't you want system add-ons to update as any other add-ons?
As for e10srollout, it stays at version 1.9 on esr channel, but could be changed if policy changed.

When backporting that fix we should, however,

It's awaiting approval for esr52 and, as many distros are affected, will, probably, be approved.

make sure the esr policy applies to us.

Are you going to switch the channel from 'release' to 'esr'?
It would help transition from ff-esr to ff in the future.
Note: Mozilla has transferred Vista and XP users to 'esr' channel in FF52.
As for e10srollout, esr policy means no e10s on Windows XP.

comment:9 Changed 19 months ago by cypherpunks

Also 1491566764500 addons.xpi DEBUG Add-on tor-launcher@torproject.org blocks e10s rollout.

comment:10 in reply to:  7 ; Changed 19 months ago by mcs

Replying to gk:

I think keeping that extension, auditing it, and making sure it does not get some out-of-bound Mozilla update is the way to go. When backporting that fix we should, however, make sure the esr policy applies to us.

Kathy and I can take this task unless you plan to work on it. We did 1/2 an audit yesterday already. We will start on this as soon as finish #21778 (I think we are close; we started working on that ticket because it seemed a little easier than #21766 and we thought doing so would help us learn what to do for #21766).

comment:11 in reply to:  9 Changed 19 months ago by mcs

Replying to cypherpunks:

Also 1491566764500 addons.xpi DEBUG Add-on tor-launcher@torproject.org blocks e10s rollout.

I see the message Add-on https-everywhere-eff@eff.org blocks e10s rollout instead, but I don't know why either extension would cause e10s to be blocked. We will investigate.

comment:12 in reply to:  10 Changed 19 months ago by gk

Replying to mcs:

Replying to gk:

I think keeping that extension, auditing it, and making sure it does not get some out-of-bound Mozilla update is the way to go. When backporting that fix we should, however, make sure the esr policy applies to us.

Kathy and I can take this task unless you plan to work on it.

Please go ahead.

comment:13 Changed 19 months ago by cypherpunks

mcs, what do you think about

[...] Torbutton WARN: DocShell is null for: https://trac.torproject.org/projects/tor/timeline

during NEWNYM?

comment:14 Changed 19 months ago by cypherpunks

The more testing is performed, the more obvious it becomes that the main reason

e10s is not enabled on Linux (and probably OS X) in ESR 52 based nightlies

is "e10s is the holy crap!".
Now it takes >400MB to open the first website (doubled). Performance degraded enormously as two processes concurrent for the same resources (no offloading of the parent). Add-ons become inadequate, GUI too. Even Mozilla realized that and asked developers to focus their efforts on WebExtensions (required for Nightly this summer) by updating https://blog.mozilla.org/addons/2017/02/16/the-road-to-firefox-57-compatibility-milestones/
In its current implementation e10s is a way to make 2 firefox.exe instead of one. Child process continues to use loopback connections, ask system DNS service and printer spooler service, have access to memory (as sandboxing will be later) and, as some idiots at Mozilla made it a memory I/O hog, try to allocate new memory objects when lacking of resources, so successfully grow to OOM by the child process and then crash with the parent (rofl:)

comment:15 in reply to:  13 Changed 18 months ago by mcs

Replying to cypherpunks:

mcs, what do you think about

[...] Torbutton WARN: DocShell is null for: https://trac.torproject.org/projects/tor/timeline

during NEWNYM?

I cannot reproduce this on OSX. What platform do you see this on? Please open a new ticket if this is consistently reproducible.

comment:16 Changed 18 months ago by mcs

Keywords: TorBrowserTeam201704R added; TorBrowserTeam201704 removed
Status: newneeds_review

Here is a patch that will cause e10s to be enabled by default, except on Windows XP (where Mozilla does not enable it by default either):

https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21876-01&id=b9af94a7458e111efa474c32bb0a6955bb5dbc8f

Since it looks like the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1348576 is not going to be backported to the ESR, Kathy and I like the simple approach we used in this patch: treat all update channels as if they are esr for e10s-related things.

We also reviewed the e10srollout system extension code as well as the code inside toolkit/mozapps/extensions/internal/E10SAddonsRollout.jsm. We did not see anything that we are concerned about: the e10srollout extension sets preferences that are then used by other parts of the Firefox code. The "esrA" policy that we will be using consists of:

  • Windows: Don't enable e10s if accessibility APIs were used recently.
  • Windows: Don't enable e10s on WindowsXP.
  • Don't enable e10s if any incompatible add-ons are installed.

There are also prefs that users can set to change the behavior, e.g., set browser.tabs.remote.force-enable to true.

comment:17 Changed 18 months ago by gk

Thanks. It seems to me we don't need the changes in install.rdf.in? They are bypassed anyway according to https://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html. We need to make sure to block system extension updates in general, though. But that we already do (learnt the hard way :) ).

comment:18 Changed 18 months ago by gk

Oh, one thing I forgot: this patch does not cause explosion on Windows if we are not shipping the sandbox and e10s there, right?

comment:19 in reply to:  17 Changed 18 months ago by mcs

Replying to gk:

Thanks. It seems to me we don't need the changes in install.rdf.in? They are bypassed anyway according to https://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html. We need to make sure to block system extension updates in general, though. But that we already do (learnt the hard way :) ).

Kathy and I forgot about that... you are correct. We have created a new patch that does not touch install.rdf.in:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21876-02&id=1240ac3e85536322d6af0075c662eaf60ab078f9

comment:20 in reply to:  18 ; Changed 18 months ago by mcs

Replying to gk:

Oh, one thing I forgot: this patch does not cause explosion on Windows if we are not shipping the sandbox and e10s there, right?

I don't think this patch will cause any new explosions, but I have not tested on Windows. This patch will enable e10s on all platforms (except WinXP) but maybe you don't want that to happen?

comment:21 in reply to:  20 Changed 18 months ago by gk

Replying to mcs:

Replying to gk:

Oh, one thing I forgot: this patch does not cause explosion on Windows if we are not shipping the sandbox and e10s there, right?

I don't think this patch will cause any new explosions, but I have not tested on Windows. This patch will enable e10s on all platforms (except WinXP) but maybe you don't want that to happen?

Well, I want to have e10s where Mozilla has it as well. But right now we compile with --disable-sandbox and I am not sure what happens in that case. They seem to be intertwined in the sense that we don't get e10s without compiling the sandbox in. So, I guess even though we enable e10s for the same user group as Mozilla it will silently fail on Windows unless we get the sandbox compilation fixed?

Last edited 18 months ago by gk (previous) (diff)

comment:22 Changed 18 months ago by gk

Okay, I applied your patch -02 to tor-browser-52.0.2esr-7.0-2 (commit 1240ac3e85536322d6af0075c662eaf60ab078f9). Feel free to close this ticket if you think the Windows problem is no problem at all.

comment:23 Changed 18 months ago by gk

Woah, we are getting e10s on Windows even without compiling the sandbox. (I tested it and hit #21766) It seems to work but is that a smart thing to have? (stability-wise etc.)

comment:24 in reply to:  23 ; Changed 18 months ago by mcs

Replying to gk:

Woah, we are getting e10s on Windows even without compiling the sandbox. (I tested it and hit #21766) It seems to work but is that a smart thing to have? (stability-wise etc.)

I didn't have a chance to comment here, but yes, the sandbox is controlled by a separate set of #ifdef's. I guess the risk is that Firefox builds with the sandbox and we do not? It seems like we have quite a few e10s-related issues on all platforms; I am not sure if Windows is worse then the others or not.

comment:25 in reply to:  24 Changed 18 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to mcs:

Replying to gk:

Woah, we are getting e10s on Windows even without compiling the sandbox. (I tested it and hit #21766) It seems to work but is that a smart thing to have? (stability-wise etc.)

I didn't have a chance to comment here, but yes, the sandbox is controlled by a separate set of #ifdef's. I guess the risk is that Firefox builds with the sandbox and we do not?

Yes. It is basically an untested combination. But after looking around a bit it should be separate enough that we can try it in the upcoming alpha I think.

comment:26 Changed 9 months ago by arthuredelstein

Keywords: tbb-no-uplift added
Note: See TracTickets for help on using tickets.