1852841 - startup Crash in [@ mozilla::freestanding::SharedSection::Layout::Resolve | ... ], part 2

Status: VERIFIED FIXED

(Firefox :: Launcher Process, defect, P2)

Description

[Ray Kraesig [:rkraesig]]

+++ This bug was initially created as a clone of Bug #1850969 +++

Startup crash in Firefox 117.0.

This has been addressed in two patches in bug 1850969; this bug exists for administrative reasons only, to ease a later potential uplift of the possibly-riskier second patch.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1842088

:gstoll, since you are the author of the regressor, bug 1842088, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

This is already fixed on m-c by Yannis' second patch landed in bug 1850969:
https://hg.mozilla.org/mozilla-central/rev/d65c86dc2295

This is more about tracking uplifts at this point. Yannis, could you attach an uplift patch to this bug so we can better track the nominations for that?

Comment 3

[Ray Kraesig [:rkraesig]]

(ni?ing, since I don't think just assigning does that.)

Comment 4

[Yannis Juglaret [:yannis]]

Attachment: D187913_beta.diff

Approval Request Comment
[Feature/Bug causing the regression]: Race condition when third-party products start threads in our main process during Firefox startup.
[User impact if declined]: For people who were crashing in bug 1850969: occasional failure to honor the dynamic DLL blocklist (when they would have crashed). For all people with third-party products starting threads early in our process: potential occasional unknown consequences of the race condition.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes, we verified that the fix does not introduce hangs for the Nightly population - as far as we know.
[Needs manual test from QE? If yes, steps to reproduce]: Yes - check that Firefox starts on Windows 10 with the usual antivirus products we test (launch Firefox at least twice for each product).
[List of other uplifts needed for the feature/fix]: The other patch from bug 1850969 (D187913) which has already been lifted to 118 beta.
[Is the change risky?]: Medium
[Why is the change risky/not risky?]: A bit risky because we are already in 118 RC and the Nightly population is not the best population to test interaction with third-party products (e.g. only five crashes from Nightly users over the last 6 months for the signatures in bug 1850969). Not so risky because the patch is simple and the potential unknown consequences of the race condition feel more risky than the patch.
[String changes made/needed]:

ESR Approval Request Comment
[Feature/Bug causing the regression]: Race condition when third-party products start threads in our main process during Firefox startup.
[User impact if declined]: For people who were crashing in bug 1850969: occasional failure to honor the dynamic DLL blocklist (when they would have crashed). For all people with third-party products starting threads early in our process: potential occasional unknown consequences of the race condition.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes, we verified that the fix does not introduce hangs for the Nightly population - as far as we know.
[Needs manual test from QE? If yes, steps to reproduce]: Yes - check that Firefox starts on Windows 10 with the usual antivirus products we test (launch Firefox at least twice for each product).
[List of other uplifts needed for the feature/fix]: The other patch from bug 1850969 (D187913) which has already been lifted to 115 ESR.
[Is the change risky?]: Medium
[Why is the change risky/not risky?]: A bit risky because we are shipping soon and the Nightly population is not the best population to test interaction with third-party products (e.g. only five crashes from Nightly users over the last 6 months for the signatures in bug 1850969). Not so risky because the patch is simple and the potential unknown consequences of the race condition feel more risky than the patch.
[String changes made/needed]:

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 5

[Donal Meehan [:dmeehan]]

Comment on attachment 9354288 [details] [diff] [review]
D187913_beta.diff

Fx118 is now in release, changing uplift request to approval-mozilla-release

Comment 6

[Daniel Bodea [:danibodea]]

QA has verified that Nightly v119.0a1 does not crash at start-up after having installed top antivirus products (ESET Smart Security Premium, Avast Premium Security, Norton AntiVirus Plus, McAfee LiveSafe, Kaspersky Internet Security, Avira, Trend) on Windows 10 and Windows 11. After having the AV installed, we launched the browser a few times and performed a spot-check.

No crashes of any kind were observed during testing. We expect this fix to land in branch 118 to also verify it in the release channel.

Comment 7

[Yannis Juglaret [:yannis]]

Comment on attachment 9354288 [details] [diff] [review]
D187913_beta.diff

On second thought, with the release of 118, it feels risky to integrate this patch directly into a potential dot release. The patch from bug 1850969 will already prevent the associated crash, so 118 should now behave mostly like we did before the patch for bug 1842088. I would rather have this attachment patch spend some time in 119 beta. Similarly, let's consider this for ESR 115.4 rather than 115.3.

Comment 8

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9354288 [details] [diff] [review]
D187913_beta.diff

Approved for 115.4esr

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/ace59fc2be9a

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

12 days ago, yannis placed a reminder on the bug using the whiteboard tag [reminder-test 2023-10-09] .

yannis, please refer to the original comment to better understand the reason for the reminder.

Comment 11

[Yannis Juglaret [:yannis]]

We haven't identified regressions for this patch so it seems fine to keep things as they are: it will be part of the 119 release and the 115.4 ESR.

Comment 12

[Daniel Bodea [:danibodea]]

QA has verified ESR 115.4esr as well; The Browser does not crash at start-up after having installed top antivirus products (ESET Smart Security Premium, Avast Premium Security, Norton AntiVirus Plus, McAfee LiveSafe, Kaspersky Internet Security, Avira, Trend) on Windows 10 and Windows 11. After having the AV installed, we performed a small exploratory, and then we launched/restarted the browser a few times.

No crashes of any kind were observed during testing.