1774155 - Windows 32-bit Crash in [@ OOM | large | NS_ABORT_OOM | mozilla::dom::DeprecationWarning]

Status: RESOLVED FIXED

(Core :: DOM: Bindings (WebIDL), defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

60 crashes from 29 installations, all with 32-builds on Windows and with Firefox 102.0b7

Pushlog: https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_102_0b6_RELEASE&tochange=FIREFOX_102_0b7_RELEASE

Could this be related to bug 1771084?

Crash report: https://crash-stats.mozilla.org/report/index/09501dbf-d018-47c6-9ee1-cd9910220614

MOZ_CRASH Reason: MOZ_CRASH(OOM)

Top 10 frames of crashing thread:

0 xul.dll NS_ABORT_OOM xpcom/base/nsDebugImpl.cpp:678
1 xul.dll mozilla::dom::DeprecationWarning dom/bindings/BindingUtils.cpp:4255
2 xul.dll mozilla::dom::CanvasRenderingContext2D_Binding::get_mozCurrentTransform dom/bindings/CanvasRenderingContext2DBinding.cpp:1993
3 xul.dll js::jit::CallDOMGetter js/src/jit/VMFunctions.cpp:1529
4 None @0x26481d31 
5 None @0x158f80b7 
6 None @0x2678295a 
7 None @0x158f7067 
8 None @0x26480538 
9 None @0x2648073a 

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Might also be related to the switch from early to late beta.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #1)

Might also be related to the switch from early to late beta.

Stack suggests this is an OOM triggered by the deprecation warning for mozCurrentTransform.

Maybe from bug 1765063: on early beta and earlier we disabled this feature, but on late beta it's a warning.

Comment 3

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

The bug is marked as tracked for firefox102 (release). However, the bug still isn't assigned.

:bhood, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 4

[Andrew McCreight [:mccr8]]

My guess would be that there are scripts on data URIs, as those can be very large. I haven't looked at this code in detail, but if it ends up copying it a bunch of times you could have problems. Looking at the crash URIs, I see a bunch that start with things like data:application/pdf;base64, which supports that theory.

Comment 5

[Andrew McCreight [:mccr8]]

I think this deprecation warning stuff is more of a DOM issue than a graphics issue, even though it is a regression from a graphics bug adding the deprecation notice. Sadly this code looks to be super inlined so it is hard to tell where exactly we're allocating the large string.

Comment 6

[Andrew McCreight [:mccr8]]

Henri suggested that this code could specifically check for a data URI and generated a fixed string like "data:/" or something.

Comment 7

[Adam Vandolder [:avandolder]]

Attachment: Bug 1774155 - Avoid copying data URI specs in deprecation warning. r?mccr8

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/91f4f59c0432
Avoid copying data URI specs in deprecation warning. r=mccr8

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/91f4f59c0432

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR102 approval when you get a chance.

Comment 11

[Adam Vandolder [:avandolder]]

[Tracking Requested - why for this release]: This crash has already occurred a significant number of times on the ESR release. Since the patch only affects the URI displayed in a deprecation warning, it is highly unlikely to result in any additional stability issues.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Tracking isn't the same thing as an approval request. Please nominate the patch for approval-mozilla-esr102:
https://bugzilla.mozilla.org/attachment.cgi?id=9289957&action=edit

Comment 13

[Adam Vandolder [:avandolder]]

Comment on attachment 9289957 [details]
Bug 1774155 - Avoid copying data URI specs in deprecation warning. r?mccr8

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: OOM crashes will continue for 102 ESR users.
• Fix Landed on Version: 105
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change only affects the URI shown in deprecation warnings for URIs with data schemes, which shouldn't affect functionality.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9289957 [details]
Bug 1774155 - Avoid copying data URI specs in deprecation warning. r?mccr8

Approved for 102.3esr.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/cb9e41e805ee

Comment 16

[Adam Vandolder [:avandolder]]

Comment on attachment 9289957 [details]
Bug 1774155 - Avoid copying data URI specs in deprecation warning. r?mccr8

Beta/Release Uplift Approval Request

• User impact if declined: OOM crashes will continue for 104 users.
• 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): This change only affects the URI shown in deprecation warnings for URIs with data schemes, which shouldn't affect functionality.
• String changes made/needed: none
• Is Android affected?: Unknown

Comment 17

[Dianna Smith [:diannaS]]

Comment on attachment 9289957 [details]
Bug 1774155 - Avoid copying data URI specs in deprecation warning. r?mccr8

Approved for 104.0.2

Comment 18

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/b702160855e9