1860767 - Don't call MOZ_ReportCrash for AutoEnterOOMUnsafeRegion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Jan de Mooij [:jandem]]

As mentioned in bug 1859737, this pulls in some unwanted machinery on Windows for dumping the stack trace. We can replace it with just a printf.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1860767 - Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r?jonco!

According to bug 1859737, there are some issues with the stack dumping code on Windows
and the goal is to stop defining MOZ_ReportCrash in non-debug builds.
For AutoEnterOOMUnsafeRegion it seems simplest to do our own printf.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea758c7008d7
Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r=jonco

Comment 3

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ea758c7008d7

Comment 4

[Yannis Juglaret [:yannis]]

Thanks for this patch [:jandem]. As noted in bug 1843354 comment 14, it would be interesting to take it for ESR, to have proper bucketing of the ESR crashes originating from AutoEnterOOMUnsafeRegion. Could you write the request if you agree?

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9359988 [details]
Bug 1860767 - Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r?jonco!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: See discussion in bug 1843354. This is a one-line low-risk patch that will improve crash signatures. The new code is also much simpler and safer.
• User impact if declined:
• Fix Landed on Version: 121
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk. Replaces some printing code on OOM with a simpler fprintf.

Comment 6

[Yannis Juglaret [:yannis]]

Adding the signatures that we expect to see go down in ESR if we take the patch.

Comment 7

[Dianna Smith [:diannaS]]

:jandem if we want to take this in ESR this cycle, we should also nominate this for beta. lmk!

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 9359988 [details]
Bug 1860767 - Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r?jonco!

Beta/Release Uplift Approval Request

• User impact if declined: This is a one-line, low-risk patch that will improve crash signatures. The new code is also much simpler and safer.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk because it's just replacing a call to a function, right before we crash on OOM, with a simpler fprintf.
• String changes made/needed:
• Is Android affected?: Yes

Comment 9

[Dianna Smith [:diannaS]]

Comment on attachment 9359988 [details]
Bug 1860767 - Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r?jonco!

Approved for 120.0b5

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/1da991d7bd0a

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9359988 [details]
Bug 1860767 - Replace MOZ_ReportCrash call for AutoEnterOOMUnsafeRegion with fprintf. r?jonco!

Approved for 115.5esr

Comment 12

[Pulsebot]

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