Closed Bug 462392 Opened 12 years ago Closed 12 years ago

RebuildAllStyleData on iframe can crash-on-unwind after flushing parent and destroying iframe

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: dbaron, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical?])

Attachments

(3 files)

Attached file simplified testcase
Yesterday Jesse Ruderman gave me an unreduced testcase (that he was having trouble reducing) that crashed in a way that I thought was debuggable with valgrind.  It turns out it was, and from the stacks I got from valgrind I wrote a more reliable and simpler testcase to exercise the same bug.

Marking [sg:critical?] given that it's dereferencing deleted objects (and potentially making virtual function calls on them).
The valgrind warning here explains the problem.

I'm not sure if the assertion showed up on Jesse's testcase, or exactly how related it is...
My testcase triggered the AllocateFrame assertion too.
Flags: blocking1.9.1?
Whiteboard: [sg:critical?]
Severity: normal → critical
I guess bug 435895 might be related to this.
There seem to be two ways a page can trigger RebuildAllStyleData:
 * location.reload() or history.go(0) within a resize event
 * triggering a media feature value change (e.g., by a resize) -- which can be flushed
I'm thinking that a few kungFuDeathGrips (on pres shells) might be the way to go here.
Attached patch patch for crashSplinter Review
This fixes the crash, but not the AllocateFrame assertion.

(I'm wondering if the AllocateFrame assertion is related to stuff allocated at a caller on the stack.)
According to valgrind leak checking with DEBUG_TRACEMALLOC_FRAMEARENA, the leak was because we leaked a rule tree.  However, it doesn't show up with XPCOM_MEM_LEAK_LOG, which is odd.
Attachment #345547 - Flags: superreview?(bzbarsky)
Attachment #345547 - Flags: review?(bzbarsky)
I think not showing up in XPCOM_MEM_LEAK_LOG is because the leak is due to the null-check in nsPresContext::FreeToShell.
Attachment #345547 - Flags: superreview?(bzbarsky)
Attachment #345547 - Flags: superreview+
Attachment #345547 - Flags: review?(bzbarsky)
Attachment #345547 - Flags: review+
So, I'm really not sure what to do about the AllocateFrame assertion.  The problem is that the pres context's pres shell pointer is null after PresShell::Destroy is called, which is when we destroy mOldRuleTree.  We could destroy mOldRuleTree during style set destruction, I suppose, but the way things are right now isn't actually doing any harm (I don't think); it's just asserting.
Duplicate of this bug: 435895
Fixed: http://hg.mozilla.org/mozilla-central/rev/5785b70454d3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
I filed bug 462788 on the leak and assertion.
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204034421 and the appropriate build on OS X.
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Group: core-security
crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7301d150bc12
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.