451323 - [FIX] Crash [@ nsAttributeTextNode::UnbindFromTree] with <optgroup> in <xbl:content>

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[Jesse Ruderman]

Attachment: testcase (makes Firefox crash on shutdown if using mallocscribble)

Steps to reproduce:
1. export MallocScribble=1
2. Run a debug build of Firefox.
3. Load the testcase.
4. Quit Firefox.

Result: crash [@ nsAttributeTextNode::UnbindFromTree], usually dereferencing 0x555555c5, but sometimes dereferencing 0xc71fe657 or nearby.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this new?  Sounds like a possible regression from bug 238072.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, it does.

Comment 3

[Johnny Stenback (:jst)]

Not blocking on this, but we'd really love a fix for this.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This should do it. Because nsAttributeTextNode is already mutationobserver, just
use NodeWillBeDestroyed.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So how are we losing our grandparent _node_ without its frame (and hence us) going away?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Some property keeps element (and its attributetextnode) alive until presshell
is destroyed. So could perhaps remove that property earlier.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That would be good (in addition to this patch), since otherwise we have a leak for the lifetime of the page, no?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Actually, the patch should be enough.
The only case when the property doesn't get removed before grandparent is 
deleted is when framemanager is about to be destroyed. In that case 
PresShell::NotifyDestroyingFrame doesn't destroy properties for individual 
frames. Properties will be destroyed right after framemanager::Destroy.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  Can we somehow assert that we're in that situation here?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

I don't think there is any easy way to assert.
When properties are deleted in Destroy, document doesn't anymore have pointer
to the relevant presshell.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Hmm.  And we can't assert that because we don't know which shell is the relevant one, I guess.  OK.

Comment 12

[Daniel Veditz [:dveditz]]

This doesn't seem to affect 1.9.0.x, and it's blamed on a bug (bug 238072) that appears to have been mozilla-central only.

If I'm wrong please renominate