crash when closing open rdf files or right click on tree after opening rdf files [@ nsXULTemplateQueryProcessorRDF::CheckIsSeparator]

VERIFIED FIXED in mozilla1.9.1a2



9 years ago
6 years ago


(Reporter: Christophe Charron, Assigned: mikekap)


(4 keywords)

1.9.0 Branch
crash, testcase, verified1.9.0.12, verified1.9.1
Dependency tree / graph
Bug Flags:
blocking1.9.0.12 +
wanted1.9.0.x +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [sg:critical?] requires bug 492037, crash signature, URL)


(2 attachments, 2 obsolete attachments)

2.07 KB, application/zip
11.26 KB, patch
Neil Deakin (mostly unavailable until September)
: review+
Neil Deakin (mostly unavailable until September)
: review+
Details | Diff | Splinter Review


9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9) Gecko/2008052906 Firefox/3.0

a xul window loading 2 rdf files in a tree.

Absolutely no problem with Flock, under windows (Mozilla/5.0 (Windows;
U; Windows NT 5.1; en-US; rv: Gecko/20080530 Firefox/ Flock/1.2.1), that is say that I could see a tree that data
in the second file rdf. Similarly, if we turn opens and closes files
via the appropriate buttons, no crash.

But with , FF3, under windows, my tree retains data in the first rdf
and if you play a little with files rdf (open and then close them and
open them by clicking on the tree etc.) on a Most likely to have
Under Ubuntu 8.04, I crash , but no proposal of sending report crashes.

Below, links to the first 4 crashes generated by this window.

Reproducible: Always

Steps to Reproduce:
1.Open a new Firefox 3
2.go to on button 1 
4.close the window with rdf file
5.right click on tree
6. do it 2 or 3 times if doesn't crash at the first time

Comment 1

9 years ago
I crashed using SeaMonkey 2.0a1pre (Gecko just by opening the URL and forcing a repaint. The result's query's processor had been deleted (mQuery->mProcessor points to memory filled with 0xdddddddd). Stack:

Ever confirmed: true


9 years ago
Component: General → XP Toolkit/Widgets: Trees
Keywords: crash, testcase
Product: Firefox → Core
QA Contact: general → xptoolkit.trees


9 years ago
Summary: crash when closing open rdf files or right click on tree after opening rdf files → crash when closing open rdf files or right click on tree after opening rdf files [@ nsXULTemplateQueryProcessorRDF::CheckIsSeparator]


9 years ago
Severity: critical → blocker
Created attachment 328514 [details]
Test case

Bug still present on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008070812 Minefield/3.1a1pre .

This is a chrome test, although I have no clue where it goes. The only reason for that is apparently the datasources attribute needs a web server and doesn't work for file:/// uri's.

The crash happens when the XPCOM garbage collector collects the query processor for the first file's results. When the results are then repainted, we get into a mess.

The old results are added to the tree when the datasources attribute is changed twice and two query processors are created, although only one reference is kept. Then when the async data for the stale file comes in, it sometimes ends up being added to the tree anyway.

The fix I found to work (for this case anyway) is to kill the (current) query processor at nsXULTemplateBuilder::LoadDataSources, although I don't know what implications that will have.
Created attachment 328516 [details] [diff] [review]

The aforementioned patch. 

I'm not entirely sure what conflicts this may cause, but this does seem fix the stale rows.
Attachment #328516 - Flags: review?(hyatt)
Attachment #328516 - Flags: review?(hyatt) → review?(enndeakin)
Comment on attachment 328516 [details] [diff] [review]

This should be a call to Uninit(PR_FALSE) to uninitialize things. I think it would be better to call this within AttributeChanged as it doesn't need to be called during Init.
Attachment #328516 - Flags: review?(enndeakin) → review-
Created attachment 328614 [details] [diff] [review]
Patch v2

Addressing Neil's comments.
Attachment #328516 - Attachment is obsolete: true
Attachment #328614 - Flags: review?(enndeakin)
Comment on attachment 328614 [details] [diff] [review]
Patch v2

Looks OK, but no observers are removed so just leave out that part of the comment.

You should also add a crashtest.
Attachment #328614 - Flags: review?(enndeakin) → review+
Created attachment 329614 [details] [diff] [review]
Patch v3 w/ semi-working crashtest

This crashtest doesn't completely work for two reasons:
 - Apparently strict_origin_policy doesn't allow loading rdf files from the same directory. Is this is a bug? (At least the other tests that load rdf files shouldn't really work)
 - The crash requires a garbage collection, so it doesn't work on release builds. It works fine for debug builds though (and will pass unconditionally on release builds).

I doubt this is good, but I don't think I can get much further than that.

Also fixed a typo.
Attachment #328614 - Attachment is obsolete: true
Attachment #329614 - Flags: review?(enndeakin)
Attachment #329614 - Flags: review?(enndeakin) → review+
Attachment #329614 - Flags: superreview?(jst)


9 years ago
Attachment #329614 - Flags: superreview?(jst) → superreview+


9 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
Keywords: checkin-needed
Whiteboard: checkin-needed
Assignee: nobody → mike.kaplinskiy
Severity: blocker → critical
Whiteboard: checkin-needed
Version: unspecified → 1.9.0 Branch
Pushed as 17107:78ac7c4b0cf1.
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2

Comment 9

8 years ago
1.9.0 !exploitable report
UNKNOWN: Read Access Violation starting at xpcom_core!PL_DHashTableOperate
Flags: in-testsuite+
Flags: blocking1.9.0.11?
Group: core-security
Flags: wanted1.9.0.x+
Whiteboard: [sg:critical?]
Flags: blocking1.9.0.11? → blocking1.9.0.11+
The patch applies to 1.9.0, but I'm not sure it's working right. With the patch I get a completely empty tree. Pressing the buttons loads the XML in an external window, but the tree in the original window is never populated.
Comment on attachment 329614 [details] [diff] [review]
Patch v3 w/ semi-working crashtest

That's the result I get in Shiretoko, too: the test page shows a blank tree instead of being populated by the datasource it set. Did Shiretoko break rdf datasources? If so I would have expected we'd have found out by now.

enn: is this behavior correct? I'm happy to land this patch if it is, but I don't want to break things and force a quick re-release. We've had enough of those lately. requesting re-review in the context of the 1.9.0 branch just as a mechanism to get your input as to whether this is appropriate for that branch.
Attachment #329614 - Flags: review?(enndeakin)
Hmmm. It seems that this patch fixes the crash but breaks the actual testcase completely. Fortunately I have a patch in the works which should fix this as well as doesn't bring back the crash.
Assignee: mike.kaplinskiy → enndeakin
Are you working on that in an existing bug? If not should I reopen this one or file a new one on the regression? Nominating for blocking1.9.1 so we don't forget the regression, but that request should be moved if there's a separate bug.
Assignee: enndeakin → mike.kaplinskiy
Flags: blocking1.9.1?
Depends on: 492037
I filed bug 492037 on the regression.
Flags: blocking1.9.1?
We'll take this fix along with the regression fix in the next 1.9.0 cycle (we're past code-freeze already and would like some nightly testing on the new fix).
Flags: blocking1.9.0.11+ → blocking1.9.0.12+
Whiteboard: [sg:critical?] → [sg:critical?] requires bug 492037


8 years ago
Depends on: 495077
Attachment #329614 - Flags: review?(enndeakin) → review+
Comment on attachment 329614 [details] [diff] [review]
Patch v3 w/ semi-working crashtest

Once the regression bug is checked in, this should be ok for 1.9.0
Keywords: fixed1.9.0.12
Was this fixed by bug 492037 or did you land this patch on 1.9.0 as well?
Verified that this is fixed in using original testcase on website and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2009063005 GranParadiso/3.0.12pre (.NET CLR 3.5.30729). crashes following test steps.
Keywords: fixed1.9.0.12 → verified1.9.0.12
Keywords: fixed1.9.1

Comment 19

8 years ago
It doesn't seem to affect 1.8.
verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre ID:20090721044139


Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20090720 Shiretoko/3.5.1pre ID:20090720042942
Keywords: fixed1.9.1 → verified1.9.1
Flags: wanted1.8.1.x-
Group: core-security
Crash Signature: [@ nsXULTemplateQueryProcessorRDF::CheckIsSeparator]
You need to log in before you can comment on or make changes to this bug.