crash calling rebuild() twice

RESOLVED FIXED in M18

Status

()

Core
XUL
P3
major
RESOLVED FIXED
18 years ago
18 years ago

People

(Reporter: Chris Waterson, Assigned: Chris Waterson)

Tracking

({crash})

Trunk
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fix in hand)

Attachments

(4 attachments)

(Assignee)

Description

18 years ago
To reproduce:

1. Save the attached test case as "test.xul"
2. Open it in a browser window.
3. Click the button
4. Click the button again. *boom*

Expected: no crash
(Assignee)

Comment 1

18 years ago
Created attachment 12125 [details]
test case
(Assignee)

Comment 2

18 years ago
The problem is that the ContentSupportMap entries hold a pointer into the
ConflictSet's arena. (Ok, I feel like I'm talking about the pyramids of Mars
layout engine right now. I'll just shut up and attach a fix.)
(Assignee)

Comment 3

18 years ago
Created attachment 12126 [details] [diff] [review]
clear content support map before clearing conflict set
(Assignee)

Comment 4

18 years ago
Ok, so some explanation is in order.

The "conflict set" is the data structure that keeps track of which of the
template rules are currently matched. Since a template rule depends on content
in the content model (e.g., if you remove a "parent" node from the tree, all the
rules that matched for that nodes children clearly don't match anymore).

The "content support map" keeps track of which rules are supported by elements
in the content model. It maps a content node to a "match".

Problem is, the conflict set owns all of the match objects. Calling
mConflictSet.Clear() in nsXULTemplateBuilder::Rebuild() causes the arena pool
that was containing all the match objects to be dumped en masse and re-created.
This leaves the content support map to be left with a dangling pointer into the
conflict set. The above test case causes us to actually try to use one of those
pointers.

The fix is to nuke the content support map before we nuke the conflict set.
Status: NEW → ASSIGNED
(Assignee)

Updated

18 years ago
Target Milestone: --- → M18

Comment 5

18 years ago
spam: Adding crash keyword...
Keywords: crash
(Assignee)

Comment 6

18 years ago
I don't like my first fix, and spent last night working up the one I'm going to 
attach shortly. I don't like it either, for reason's I'll explain 
momentarily...
(Assignee)

Comment 7

18 years ago
Created attachment 12240 [details] [diff] [review]
remove ContentSupportMap
(Assignee)

Comment 8

18 years ago
The attached patch (besides commenting a bunch of junk) removes the 
ContentSupportMap structure. This structure was used to implement 
CreateTemplateContents(): it recorded the match from whence each piece of 
generated content was created so that we could quickly pick up and resume 
template building.

I thought that removing it would be a good thing to do, unfortunately, it slows 
folder opening down by about 7%. I thought I'd shove it into the bug for 
posterity's sake (or maybe to deal with later).
(Assignee)

Comment 9

18 years ago
Created attachment 12244 [details] [diff] [review]
like the first patch, but better.
(Assignee)

Updated

18 years ago
Keywords: nsbeta3
Whiteboard: fix in hand
(Assignee)

Comment 10

18 years ago
fix checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.