Closed
Bug 1256419
Opened 8 years ago
Closed 8 years ago
crashes in nsDOMWindowList::EnsureFresh
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | + | fixed |
firefox48 | --- | fixed |
People
(Reporter: dbaron, Assigned: bzbarsky)
References
Details
(Keywords: crash, topcrash)
Crash Data
Attachments
(2 files)
2.81 KB,
patch
|
smaug
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
1.96 KB,
patch
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]: This bug was filed from the Socorro interface and is report bp-b6ab14ac-1f9e-4b4b-9a72-12d092160314. ============================================================= There have been a decent number of crashes in nsDOMWindowList::EnsureFresh. Not sure if it's quite enough to qualify as a topcrash, but it's at least close. They first occurred in builds from 2016-02-10. See query at: https://crash-stats.mozilla.com/signature/?product=Firefox&release_channel=nightly&platform=Windows&date=%3E%3D2016-01-01&signature=nsDOMWindowList%3A%3AEnsureFresh&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&page=1#aggregations (40 crashes so far on nightly). At first glance, it looks like a missing null-check in code added in bug 1245554, although I didn't actually do the math to double-check that an 0x10 crash address matches childWindows being null.
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 1•8 years ago
|
||
0x10 (or 0x8 in 32-bit builds, which also shows up in the reports) is in fact consistent with that: there's one word of vtable, then one word of refcount, then the mDocShellNode member we're trying to read. And yes, this needs a null-check.
Assignee: nobody → bzbarsky
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8730369 -
Flags: review?(peterv)
Assignee | ||
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8730369 [details] [diff] [review] Null-check our nsDOMWindowList before trying to get its length Approval Request Comment [Feature/regressing bug #]: Bug 1245554 [User impact if declined]: Crashes sometimes [Describe test coverage new/current, TreeHerder]: we have good coverage for the non-crashing case. [Risks and why]: Extremely low risk. Just skips some code if we don't have the right object around anymore. [String/UUID change made/needed]: None.
Attachment #8730369 -
Flags: approval-mozilla-aurora?
Tracked since it's a top crasher.
Comment 6•8 years ago
|
||
Comment on attachment 8730369 [details] [diff] [review] Null-check our nsDOMWindowList before trying to get its length Just got this. Looks like the old code did have a null check.
Attachment #8730369 -
Flags: review?(peterv) → review+
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f75d479d9d51
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
I want to wait to see crash-stats on this one confirm that the fix is helping before uplifting it even though the fix is a simple not null check.
Comment on attachment 8730369 [details] [diff] [review] Null-check our nsDOMWindowList before trying to get its length Crash-stats does not show any occurrences of this signature on Nightly 48 since 03-15 build, looks good. Aurora47+
Attachment #8730369 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/dcc8b840bd8f
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•