Closed Bug 1275129 Opened 8 years ago Closed 8 years ago

Crash in xpc::SizeOfTreeIncludingThis

Categories

(Core :: DOM: Core & HTML, defect)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-e0b7e620-4d4a-4206-991e-3be932160517.
=============================================================

xpc::SizeOfTreeIncludingThis() doesn't allow that SubtreeRoot() might return null.

23 crashes like this in the past 7 days, all in 48.0a2.
Comment on attachment 8755655 [details] [diff] [review]
Add missing null check in OrphanReporter::sizeOfIncludingThis

Review of attachment 8755655 [details] [diff] [review]:
-----------------------------------------------------------------

This method isn't supposed to return null, but I could see that this particular code might end up accessing an unlinked node.
Attachment #8755655 - Flags: review?(continuation) → review+
Depends on: 1266630
https://hg.mozilla.org/integration/mozilla-inbound/rev/73bb17e67fc160729a66be353d968350e37def61
Bug 1275129 - Add missing null check in OrphanReporter::sizeOfIncludingThis. r=mccr8.
Comment on attachment 8755655 [details] [diff] [review]
Add missing null check in OrphanReporter::sizeOfIncludingThis

Approval Request Comment
[Feature/regressing bug #]: memory reporting

[User impact if declined]: crashes when measuring memory in about:memory.

[Describe test coverage new/current, TreeHerder]: none.

[Risks and why]: Negligible. It's a single additional null check.

[String/UUID change made/needed]: None.
Attachment #8755655 - Flags: approval-mozilla-beta?
Attachment #8755655 - Flags: approval-mozilla-aurora?
SubtreeRoot should never return null.
nsINode::~nsINode() has 
MOZ_ASSERT(mSubtreeRoot == this, "Didn't restore state properly?");

Are we missing to call UnbindFromTree somewhere?
https://hg.mozilla.org/mozilla-central/rev/73bb17e67fc1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8755655 [details] [diff] [review]
Add missing null check in OrphanReporter::sizeOfIncludingThis

I see 0 instances of this on Beta47. I'd prefer to let this one ride the Aurora48 to Beta48 train.
Attachment #8755655 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8755655 [details] [diff] [review]
Add missing null check in OrphanReporter::sizeOfIncludingThis

Crash fix, Aurora48+
Attachment #8755655 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.