1597543 - Crash in [@ JS::AddAssociatedMemory]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Wayne Mery (:wsmwk)]

I've recently seen several of these in "idle" tabs.

This bug is for crash report bp-e843afd4-5f36-4cd8-81b6-eb7730191118.

Top 10 frames of crashing thread:

0 xul.dll JS::AddAssociatedMemory js/src/jsapi.cpp:1230
1 xul.dll void mozilla::dom::BindingJSObjectCreator<mozilla::dom::CanvasRenderingContext2D>::CreateObject dom/bindings/BindingUtils.h:2609
2 xul.dll mozilla::dom::CanvasRenderingContext2D_Binding::Wrap dom/bindings/CanvasRenderingContext2DBinding.cpp:7642
3 xul.dll mozilla::dom::CanvasRenderingContext2D::WrapObject dom/canvas/CanvasRenderingContext2D.cpp:997
4 xul.dll mozilla::dom::XPCOMObjectToJsval dom/bindings/BindingUtils.cpp:1123
5 xul.dll static bool mozilla::dom::HTMLCanvasElement_Binding::getContext dom/bindings/HTMLCanvasElementBinding.cpp:293
6 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3236
7 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:539
8 xul.dll Interpret js/src/vm/Interpreter.cpp:3084
9 xul.dll js::RunScript js/src/vm/Interpreter.cpp:424

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1597543 - Don't try and associate memory if we failed to create the reflector object r?bzbarsky

The problem is we're still calling JS::AddAssociatedMemory with a null object pointer. There's an asssertion for this in the JS engine but this doesn't happen in our tests.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/668e9d7ed1b1
Don't try and associate memory if we failed to create the reflector object r=bzbarsky

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/668e9d7ed1b1

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta/ESR? It grafts cleanly as-landed.

Comment 5

[Jon Coppeard (:jonco)]

Comment on attachment 9109974 [details]
Bug 1597543 - Don't try and associate memory if we failed to create the reflector object r?bzbarsky

Beta/Release Uplift Approval Request

• User impact if declined: Possible crash on OOM.
• 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 is a very simple fix to avoid a null-pointer deref on OOM.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a crash.
• User impact if declined: Possible crash on OOM.
• Fix Landed on Version: 72
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very simple fix to avoid a null-pointer deref on OOM.
• String or UUID changes made by this patch:

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9109974 [details]
Bug 1597543 - Don't try and associate memory if we failed to create the reflector object r?bzbarsky

Low risk patch for some rare crashes, patch is not changing logic and looks low risk, approved for our last beta and esr, thanks.

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/040f9f23179e

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/3bf7df179a7b