1677036 - Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Mike Hommey [:glandium]]

Subprocesses of rosetta processed default to run under rosetta, so when the old x86_64 version executes the upgraded version, it still runs as x86_64 even when the upgraded version is universal.

Comment 1

[Mike Hommey [:glandium]]

We restart through https://searchfox.org/mozilla-central/rev/5a1a34953a26117f3be1a00db20c8bbdc03273d6/toolkit/xre/MacLaunchHelper.mm#21-40, which uses NSTask, which doesn't have a way to specify the wanted architecture (cf. https://twitter.com/kubamracek/status/1276222055264862209). The proposed command line tool (/usr/bin/arch) is not really convenient, because it relies on the exact architecture in the binary that is being executed (e.g. arm64 vs. arm64e (Big Sur comes with arm64e system executables)).

Haik, how do you feel about switching to posix_spawn, at least for the upgrade-initiated process (re)launch?

Comment 2

[Mike Hommey [:glandium]]

(there's also the question whether it's worth doing)

Comment 3

[Haik Aftandilian [:haik]]

I think using /usr/bin/arch is the safest way to address this for now given that we previously changed this code from posix_spawn to use NSTask to workaround problems. CC'ing Markus and Stephen who may have input on this.

We can change LaunchChildMac() to check if its running under Rosetta and if so, launch under arch with /usr/bin/arch -arm64 -x86_64 .... That will automatically fall back to x64 if the binary is not universal for some reason and we can make the code be #ifdef x64 and only used when running under Rosetta to limit risk to normal x64 updates.

Comment 4

[Stephen A Pohl [:spohl]]

(In reply to Haik Aftandilian [:haik] from comment #3)

We can change LaunchChildMac() to check if its running under Rosetta and if so, launch under arch with /usr/bin/arch -arm64 -x86_64 .... That will automatically fall back to x64 if the binary is not universal for some reason and we can make the code be #ifdef x64 and only used when running under Rosetta to limit risk to normal x64 updates.

This would be my preferred approach as well. This only impacts the first launch after an update though, right? Subsequent runs of Firefox will run the ARM version?

Comment 5

[Haik Aftandilian [:haik]]

(In reply to Stephen A Pohl [:spohl] from comment #4)

(In reply to Haik Aftandilian [:haik] from comment #3)

We can change LaunchChildMac() to check if its running under Rosetta and if so, launch under arch with /usr/bin/arch -arm64 -x86_64 .... That will automatically fall back to x64 if the binary is not universal for some reason and we can make the code be #ifdef x64 and only used when running under Rosetta to limit risk to normal x64 updates.

This would be my preferred approach as well. This only impacts the first launch after an update though, right? Subsequent runs of Firefox will run the ARM version?

Yes, if the user launches Firefox from the Finder/Dock the OS should choose the arm64 version.

Comment 6

[Stephen A Pohl [:spohl]]

So this should only affect very early adopters of Apple Silicon before our universal bundles make it to release, and only for a first restart after an update... This may not be worth a patch (as pointed out in comment 2), even if low risk. Especially considering that this patch would have to be uplifted to the version prior to the universal build to relaunch the ARM version.

Comment 7

[Mike Hommey [:glandium]]

Especially considering that this patch would have to be uplifted to the version prior to the universal build to relaunch the ARM version.

It wouldn't have to. People who never quit Firefox would go x86_64 -> unpatched universal but x86_64 -> universal bug x86_64 -> universal arm64

Comment 8

[Stephen A Pohl [:spohl]]

(In reply to Mike Hommey [:glandium] from comment #7)

Especially considering that this patch would have to be uplifted to the version prior to the universal build to relaunch the ARM version.

It wouldn't have to. People who never quit Firefox would go x86_64 -> unpatched universal but x86_64 -> universal bug x86_64 -> universal arm64

Oh right. Then yes, a low-risk patch would work great here.

Comment 9

[Haik Aftandilian [:haik]]

I'll work on the fix as described on comment 3 and we can go from there.

Comment 10

[Haik Aftandilian [:haik]]

Attachment: Bug 1677036 - Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64 r?spohl!

Switch to arm64 native execution when updating from an emulated browser instance.

Comment 11

[[e-mail] (:bhearsum)]

(In reply to Mike Hommey [:glandium] from comment #0)

Subprocesses of rosetta processed default to run under rosetta, so when the old x86_64 version executes the upgraded version, it still runs as x86_64 even when the upgraded version is universal.

What happens the next time the user starts Firefox themselves? Does it switch over to arm64 at that point?

Comment 12

[Haik Aftandilian [:haik]]

(In reply to [e-mail] (:bhearsum) from comment #11)

(In reply to Mike Hommey [:glandium] from comment #0)

Subprocesses of rosetta processed default to run under rosetta, so when the old x86_64 version executes the upgraded version, it still runs as x86_64 even when the upgraded version is universal.

What happens the next time the user starts Firefox themselves? Does it switch over to arm64 at that point?

Yes. If the user quits Firefox, the next start will be native. It's only a problem if they only ever update and never manually quit.

Comment 13

[Haik Aftandilian [:haik]]

I'm running into a problem with this approach. When testing updating an x64 build to a universal build, the problem I'm finding is that GetArchitecturesForBinary() seems to return a stale result after Nightly.app/Contents/firefox has been updated. Ignoring the arch check and unconditionally executing the update with arch -arm64 -x86_64 still launches it as x64. It's as if the binary status of Nightly.app/Contents/firefox doesn't get updated during the update/restart process. This continues on repeated updates.

(Previous testing didn't uncover this because I used a universal build run under arch -x86_64.)

Comment 14

[Haik Aftandilian [:haik]]

With help from the update team, we realized the problem with the earlier version was that it was trying to change architectures at the wrong point. MacLaunchHelper.mm:LaunchChildMac() is used to restart the browser before the update is applied and the browser is started again using the new binaries. Instead we need to switch architectures on the final browser startup call which is launchchild_osx.mm:LaunchChild().

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fef817d1c02
Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64 r=spohl,bytesized

Comment 16

[Dorel Luca [:dluca]]

Backed out changeset 4fef817d1c02 (bug 1677036) for OSX build bustage in mozapps/update/updater/launchchild_osx. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=323865604&repo=autoland&lineNumber=55895

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=4fef817d1c02491a460049c50156828a2e918717

Backout:
https://hg.mozilla.org/integration/autoland/rev/331ca44de625509624cfcef5695f63108f622e4c

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/1a164819adef
Backed out changeset 4fef817d1c02 for OSX build bustage in mozapps/update/updater/launchchild_osx. CLOSED TREE

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84a86222614e
Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64 r=spohl,bytesized

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/84a86222614e

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Per discussion with Haik, we're going to let this ride 85 rather than trying to uplift this late in the cycle.

Comment 21

[Mike Hommey [:glandium]]

Should we uplift this to esr78 somewhere between now and july (for esr91)?

Comment 22

[Haik Aftandilian [:haik]]

Comment on attachment 9190732 [details]
Bug 1677036 - Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64 r?spohl!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This automatically switches to native mode when a user updates from an Intel build (such as ESR78) to an update that is a universal binary.
• User impact if declined: When users running ESR 78, which is an Intel build, on Apple Silicon devices upgrade to the next ESR which will have native Apple Silicon support, the update won't automatically start running natively. The update will run emulated until the user restarts the browser.
• Fix Landed on Version: 85
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The changes affect update which has some risk. But the changes only change the behavior when running under Rosetta and updating to a universal binary. i.e., only Apple Silicon users running the ESR should be affected by the change.
• String or UUID changes made by this patch:

Comment 23

[Haik Aftandilian [:haik]]

(In reply to Mike Hommey [:glandium] from comment #21)

Should we uplift this to esr78 somewhere between now and july (for esr91)?

Yes, I think so. It's worth uplifting so that users on ESR on Apple Silicon have a seamless switch to native mode when updating to the next ESR.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to ESR, but since we're not in a hurry here, let's let it bake on Release for a cycle first.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9190732 [details]
Bug 1677036 - Upgrading from x86_64 mac build to universal build on Apple Silicon keeps running x86_64 r?spohl!

No issues reported with this so far. Approved for 78.8esr.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/f1c3dd3d8bc2