1530770 - (android-e10s-multi) [Meta] Enable Android e10s-multi

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, enhancement, P3)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Right now we only support a single content process. We should support more than that.

Comment 1

[Chris Peterson [:cpeterson]]

P1. Important for crash resilience and Fission, but not needed for Fenix MVP.

Comment 2

[Chris Peterson [:cpeterson]]

James says that we should enable e10s-multi for Fenix MVP.

Comment 3

[Chris Peterson [:cpeterson]]

Assigning to Bobby because James says Bobby's working on e10s-multi. This is a nice-to-have for Fenix MVP.

Comment 4

[Bobby Holley (:bholley)]

I've rebased snorp's patch and debugged it a bit. The main problem causing the test failures is that we're asked to create more than three content processes, which we can't do, and then just hang. This happens because the parent spawns three child processes and then shuts a few of them down, leading it to believe it has runway to start more.

AFAICT There are two problems in the code that lead to this condition:
(1) At present, the standard shutdown path in the child process doesn't seem to actually cause the process to be terminated such that we receive onServiceDisconnected in the parent. I think if this were an opt build we'd eventually terminate via the ForceKillTimer (which is disabled for debug builds), but then we'd still hit the second problem:
(2) It's racey, because child process shutdown is fundamentally asynchronous. We'll need to handle this better.

Let's tackle (1) first. I've traced us all the way to returning from XRE_InitChildProcess, which returns out of nativeRun. But the java portion of the app continues to run. What's the right way for this to be hooked up?

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Bobby Holley (:bholley) from comment #4)

I've rebased snorp's patch and debugged it a bit. The main problem causing the test failures is that we're asked to create more than three content processes, which we can't do, and then just hang. This happens because the parent spawns three child processes and then shuts a few of them down, leading it to believe it has runway to start more.

AFAICT There are two problems in the code that lead to this condition:
(1) At present, the standard shutdown path in the child process doesn't seem to actually cause the process to be terminated such that we receive onServiceDisconnected in the parent. I think if this were an opt build we'd eventually terminate via the ForceKillTimer (which is disabled for debug builds), but then we'd still hit the second problem:

Ah. The way we handle this in GeckoRuntime is to listen[0] for the Gecko:Exited message. We need to do something similar in GeckoServiceChildProcess and then stop the service.

(2) It's racey, because child process shutdown is fundamentally asynchronous. We'll need to handle this better.

I thought we had something hacked up to just kill children in order to avoid this, but maybe I'm misremembering?

Let's tackle (1) first. I've traced us all the way to returning from XRE_InitChildProcess, which returns out of nativeRun. But the java portion of the app continues to run. What's the right way for this to be hooked up?

[0] https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java#151

Comment 6

[Chris Peterson [:cpeterson]]

Adding [geckoview:fenix:m7] whiteboard tag because we want e10s-multi soon but it's not strictly a Fenix MVP release blocker.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

I spoke with James and I recommend explicitly not targeting MVP for this as it adds risk.

Comment 8

[Chris Peterson [:cpeterson]]

Deferring this bug from Fenix's M7 (July) milestone to the M8 backlog for later in Q3.

Comment 9

[Chris Peterson [:cpeterson]]

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

Comment 10

[Chris Peterson [:cpeterson]]

Fenix issue requesting e10s-multi so one tab crash doesn't crash every tab:
https://github.com/mozilla-mobile/fenix/issues/2863

Comment 11

[Chris Peterson [:cpeterson]]

Defer e10s-multi to October because we may get more engineering help then.

Comment 12

[Chris Peterson [:cpeterson]]

Aaron will start working on Android e10s-multi in October, but we expect e10s-multi to be a multi-month project.

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

e10s-multi rode in 90. Resolving.

Comment 14

[Chris Peterson [:cpeterson]]

Moving Android Fission bugs to the new GeckoView::Sandboxing component.