Last Comment Bug 368627 - Problem with XForms using itemsets across models
: Problem with XForms using itemsets across models
Status: RESOLVED FIXED
: fixed1.8.0.12, fixed1.8.1.4
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: x86 Windows XP
: -- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (reviewing overload)
: Stephen Pride
:
Mentors:
Depends on:
Blocks: 353738
  Show dependency treegraph
 
Reported: 2007-01-29 15:20 PST by Alex Bleasdale
Modified: 2016-07-15 14:46 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (2.04 KB, application/xhtml+xml)
2007-01-30 01:37 PST, alexander :surkov
no flags Details
refcounting (for 1.8) (7.03 KB, patch)
2007-01-30 06:33 PST, Olli Pettay [:smaug] (reviewing overload)
no flags Details | Diff | Splinter Review
proposed patch (9.63 KB, patch)
2007-01-30 10:53 PST, Olli Pettay [:smaug] (reviewing overload)
aaronr: review+
surkov.alexander: review+
Details | Diff | Splinter Review

Description Alex Bleasdale 2007-01-29 15:20:13 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1

The code below - on adding an item to an XForms itemset - causes firefox to exit.  Can be reproduced on both FF1.5 and FF2.0 using the latest versions of the XForms addon (0.7x).  

We suspect this is because the referent and the itemset appear in different models.

Error offset: 000059d2

Example code pasted below:
--------------------------------
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:xf="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
	<head>
		<xf:model id="default">
			<xf:instance xmlns="" id="CRV-data">
				<data>
					<items>
						<item>item 1</item>
						<item>item 2</item>
						<item>item 3</item>
					</items>
				</data>
			</xf:instance>
		</xf:model>
		<xf:model id="code-tables">
			<xf:instance xmlns="" id="ColorCode">
				<CodeTable>
					<DataElementName>ColorCode</DataElementName>
					<EnumeratedValues>
						<Item>
							<Label>Red</Label>
							<Value>red</Value>
						</Item>
						<Item>
							<Label>Green</Label>
							<Value>green</Value>
						</Item>
						<Item>
							<Label>Blue</Label>
							<Value>blue</Value>
						</Item>
					</EnumeratedValues>
				</CodeTable>
			</xf:instance>
		</xf:model>
	</head>
	<body>
		<xf:group model="default" ref="/data/items">
			<xf:label>Group Label for item: </xf:label>
			<xf:repeat model="default" nodeset="/data/items">
				<xf:input ref="item">
					<xf:label>Item:</xf:label>
				</xf:input>
				<xf:select1 ref="item">
					<xf:label>Item Code:</xf:label>
					<xf:itemset model="code-tables" nodeset="instance('ColorCode')/EnumeratedValues/Item">
						<xf:label ref="Label" />
						<xf:value ref="Value" />
					</xf:itemset>
				</xf:select1>
			</xf:repeat>
			<xf:trigger>
				<xf:label>Add Item</xf:label>
				<xf:action ev:event="DOMActivate">
					<xf:insert nodeset="/data/items[last()]" position="after" at="last()" />
				</xf:action>
			</xf:trigger>
		</xf:group>
	</body>
</html>


Reproducible: Always

Steps to Reproduce:
1. Paste code into xml document and run locally
2. Click "Add Item"
3. Firefox xforms.dll should crash
Actual Results:  
Firefox exits.

Expected Results:  
Another item should have been added to the itemlist.

<html xmlns="http://www.w3.org/1999/xhtml" xmlns:xf="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
	<head>
		<xf:model id="default">
			<xf:instance xmlns="" id="CRV-data">
				<data>
					<items>
						<item>item 1</item>
						<item>item 2</item>
						<item>item 3</item>
					</items>
				</data>
			</xf:instance>
		</xf:model>
		<xf:model id="code-tables">
			<xf:instance xmlns="" id="ColorCode">
				<CodeTable>
					<DataElementName>ColorCode</DataElementName>
					<EnumeratedValues>
						<Item>
							<Label>Red</Label>
							<Value>red</Value>
						</Item>
						<Item>
							<Label>Green</Label>
							<Value>green</Value>
						</Item>
						<Item>
							<Label>Blue</Label>
							<Value>blue</Value>
						</Item>
					</EnumeratedValues>
				</CodeTable>
			</xf:instance>
		</xf:model>
	</head>
	<body>
		<xf:group model="default" ref="/data/items">
			<xf:label>Group Label for item: </xf:label>
			<xf:repeat model="default" nodeset="/data/items">
				<xf:input ref="item">
					<xf:label>Item:</xf:label>
				</xf:input>
				<xf:select1 ref="item">
					<xf:label>Item Code:</xf:label>
					<xf:itemset model="code-tables" nodeset="instance('ColorCode')/EnumeratedValues/Item">
						<xf:label ref="Label" />
						<xf:value ref="Value" />
					</xf:itemset>
				</xf:select1>
			</xf:repeat>
			<xf:trigger>
				<xf:label>Add Item</xf:label>
				<xf:action ev:event="DOMActivate">
					<xf:insert nodeset="/data/items[last()]" position="after" at="last()" />
				</xf:action>
			</xf:trigger>
		</xf:group>
	</body>
</html>
Comment 1 alexander :surkov 2007-01-30 01:37:08 PST
Created attachment 253307 [details]
testcase
Comment 2 aaronr 2007-01-30 02:40:01 PST
well, we shouldn't crash, that is for sure :-).  Looks like we are finding some 'bad' nodes (probably already freed nodes) on the model's control list.  We are going through the model refreshing the controls (post rebuild since this was due to an insert) and we reach a controlListItem whose mNode is bad.  The whole controlListItem looks questionable for that matter.  Lots of junk data in it.
Comment 3 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 05:21:08 PST
Couldn't reproduce using Linux/Trunk and the testcase.
Comment 4 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 05:29:08 PST
Using 1.8 does crash.
Comment 5 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 05:51:06 PST
So my guess is that nsXFormsControlListItem object gets deleted while it
is used in nsXFormsModelElement::RefreshSubTree.
We could probably use refcounting for nsXFormsControlListItems.
Comment 6 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 06:33:40 PST
Created attachment 253327 [details] [diff] [review]
refcounting (for 1.8)

Aaron, what do you think about this approach.
I don't quite like manually AddReffing/Releasing, but
that was the easiest way to do it, or at least to test that it
fixes the crash.
Comment 7 Alex Bleasdale 2007-01-30 07:15:15 PST
For a workaround (and for anyone who has a similar problem) - if you merge the  referent and the itemset into the same model, the xform will work without crashing.
Comment 8 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 09:49:00 PST
Comment on attachment 253327 [details] [diff] [review]
refcounting (for 1.8)

I'll use this approach but just clean up the patch a bit.
Comment 9 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 10:53:18 PST
Created attachment 253359 [details] [diff] [review]
proposed patch

Can you test whether this fixes the bug in Windows too, just to make sure.
Comment 10 Olli Pettay [:smaug] (reviewing overload) 2007-01-30 10:53:59 PST
Comment on attachment 253359 [details] [diff] [review]
proposed patch

The patch was made using 1.8, but should apply to trunk too (with some fuzz)
Comment 11 aaronr 2007-01-30 13:26:23 PST
(In reply to comment #9)
> Created an attachment (id=253359) [details]
> proposed patch
> 
> Can you test whether this fixes the bug in Windows too, just to make sure.
> 

This patch fixes my Windows trunk crash.
Comment 12 aaronr 2007-01-30 14:05:12 PST
Comment on attachment 253359 [details] [diff] [review]
proposed patch

not a huge fan of the addref stuff myself, but I think in this case this approach will save us from having to debug other control list management problems in the future.
Comment 13 alexander :surkov 2007-01-31 08:53:27 PST
Comment on attachment 253359 [details] [diff] [review]
proposed patch

Looks ok
Comment 14 aaronr 2007-04-23 16:02:07 PDT
checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16

Note You need to log in before you can comment on or make changes to this bug.