Closed
Bug 1515463
Opened 2 years ago
Closed 2 years ago
Crash in nsWrapperCache::GetWrapperMaybeDead
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | wontfix |
firefox65 | --- | verified |
firefox66 | --- | verified |
People
(Reporter: marcia, Assigned: smaug)
Details
(Keywords: crash, regression, regressionwindow-wanted)
Crash Data
Attachments
(1 file)
760 bytes,
patch
|
peterv
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-e3de76ab-519c-493d-b034-0c7a30181213. ============================================================= Seen while looking at nightly crash stats: https://bit.ly/2QKC07v. Mac crash which as 13 crashes/5 installs in the last 7 days. No comments and the URLs don't offer much in the way of clues. It appears this regression goes back to at least when 64 was in nightly. Top 10 frames of crashing thread: 0 XUL nsWrapperCache::GetWrapperMaybeDead const dom/base/nsWrapperCache.h:162 1 XUL nsWrapperCache::GetWrapper const dom/base/nsWrapperCacheInlines.h:14 2 XUL mozilla::dom::MessageBroadcaster_Binding::getChildAt dom/bindings/BindingUtils.h:959 3 XUL bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3064 4 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:443 5 XUL js::ForwardingProxyHandler::call const js/src/vm/Interpreter.cpp:606 6 XUL js::CrossCompartmentWrapper::call const js/src/proxy/CrossCompartmentWrapper.cpp:304 7 XUL js::Proxy::call js/src/proxy/Proxy.cpp:535 8 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:509 9 XUL js::jit::DoCallFallback js/src/vm/Interpreter.cpp:594 =============================================================
Comment 2•2 years ago
|
||
The two crashes I see in the last week are in mozilla::dom::MessageBroadcaster_Binding::getChildAt(), like the original crash.
Updated•2 years ago
|
Component: General → DOM
Comment 3•2 years ago
|
||
All three are also cross compartment calls, so presumably that's related.
Assignee | ||
Comment 4•2 years ago
|
||
bug 888600 landed already 61. Peterv might have the webidl + MM setup better in mind.
Flags: needinfo?(bugs) → needinfo?(peterv)
Assignee | ||
Comment 5•2 years ago
|
||
oh, hmm, is this just about https://searchfox.org/mozilla-central/rev/0ee0b63732d35d16ba22d5a1120622e2e8d58c29/dom/chrome-webidl/MessageManager.webidl#545 not allowing return null.
Assignee | ||
Comment 6•2 years ago
|
||
Most probably. At least window.messageManager.getChildAt(100); in Scratchpad (browser) crashes.
Assignee: nobody → bugs
Assignee | ||
Comment 7•2 years ago
|
||
Attachment #9034083 -
Flags: review?(peterv)
Updated•2 years ago
|
Attachment #9034083 -
Flags: review?(peterv) → review+
Assignee | ||
Updated•2 years ago
|
Flags: needinfo?(peterv)
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e5f8f958a17f MessageBroadcaster.getChildAt should be out-of-bounds safe, r=peterv
Comment 9•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e5f8f958a17f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Comment 10•2 years ago
|
||
Please request Beta approval on this when you get a chance.
Assignee | ||
Comment 11•2 years ago
|
||
Comment on attachment 9034083 [details] [diff] [review] mm_null_child.diff [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 888600 User impact if declined: Crash if some privileged Js code passes invalid index to MessageBroadcaster.getChildAt Is this code covered by automated tests?: No Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: (https://bugzilla.mozilla.org/show_bug.cgi?id=1515463#c6) List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): String changes made/needed:
Flags: needinfo?(bugs)
Attachment #9034083 -
Flags: approval-mozilla-beta?
Comment 12•2 years ago
|
||
Comment on attachment 9034083 [details] [diff] [review] mm_null_child.diff [Triage Comment] Simple null check crash fix. Approved for 65.0b9.
Attachment #9034083 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•2 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/5c4518a4eb1f
Updated•2 years ago
|
Flags: qe-verify+
Comment 14•2 years ago
|
||
I successfully reproduced the issue on Firefox Nightly 66.0a1 (2018-12-11) under macOS 10.12 using the information found in Comment 6 and some help from Olli.
The issue is not reproducible anymore on Firefox Beta 65.0b9 and latest Nightly 66.0a1 (2019-01-08) under macOS 10.12, Windows 10 (x64) and Ubuntu 18.04 (x64).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•2 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•