Last Comment Bug 441785 - crash when closing open rdf files or right click on tree after opening rdf files [@ nsXULTemplateQueryProcessorRDF::CheckIsSeparator]
: crash when closing open rdf files or right click on tree after opening rdf fi...
[sg:critical?] requires bug 492037
: crash, testcase, verified1.9.0.12, verified1.9.1
Product: Core
Classification: Components
Component: XUL (show other bugs)
: 1.9.0 Branch
: x86 All
-- critical (vote)
: mozilla1.9.1a2
Assigned To: Mike Kaplinskiy [:mikekap] (at school, no time)
: Neil Deakin
Depends on: 492037 495077
  Show dependency treegraph
Reported: 2008-06-25 05:10 PDT by Christophe Charron
Modified: 2011-06-13 10:01 PDT (History)
13 users (show)
dveditz: blocking1.9.0.12+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Test case (2.07 KB, application/zip)
2008-07-08 10:19 PDT, Mike Kaplinskiy [:mikekap] (at school, no time)
no flags Details
Patch (967 bytes, patch)
2008-07-08 10:21 PDT, Mike Kaplinskiy [:mikekap] (at school, no time)
enndeakin: review-
Details | Diff | Splinter Review
Patch v2 (935 bytes, patch)
2008-07-08 20:03 PDT, Mike Kaplinskiy [:mikekap] (at school, no time)
enndeakin: review+
Details | Diff | Splinter Review
Patch v3 w/ semi-working crashtest (11.26 KB, patch)
2008-07-14 21:25 PDT, Mike Kaplinskiy [:mikekap] (at school, no time)
enndeakin: review+
enndeakin: review+
jst: superreview+
Details | Diff | Splinter Review

Description User image Christophe Charron 2008-06-25 05:10:27 PDT
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 User image 2008-06-25 06:30:27 PDT
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:

Comment 2 User image Mike Kaplinskiy [:mikekap] (at school, no time) 2008-07-08 10:19:57 PDT
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.
Comment 3 User image Mike Kaplinskiy [:mikekap] (at school, no time) 2008-07-08 10:21:20 PDT
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.
Comment 4 User image Neil Deakin 2008-07-08 19:12:04 PDT
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.
Comment 5 User image Mike Kaplinskiy [:mikekap] (at school, no time) 2008-07-08 20:03:16 PDT
Created attachment 328614 [details] [diff] [review]
Patch v2

Addressing Neil's comments.
Comment 6 User image Neil Deakin 2008-07-09 11:22:56 PDT
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.
Comment 7 User image Mike Kaplinskiy [:mikekap] (at school, no time) 2008-07-14 21:25:55 PDT
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.
Comment 8 User image Reed Loden [:reed] (use needinfo?) 2008-08-19 21:17:12 PDT
Pushed as 17107:78ac7c4b0cf1.
Comment 9 User image Bob Clary [:bc:] 2009-04-28 16:14:46 PDT
1.9.0 !exploitable report
UNKNOWN: Read Access Violation starting at xpcom_core!PL_DHashTableOperate
Comment 10 User image Daniel Veditz [:dveditz] 2009-05-07 01:17:15 PDT
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 11 User image Daniel Veditz [:dveditz] 2009-05-07 09:59:53 PDT
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.
Comment 12 User image Neil Deakin 2009-05-07 13:42:16 PDT
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.
Comment 13 User image Daniel Veditz [:dveditz] 2009-05-07 15:13:40 PDT
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.
Comment 14 User image Neil Deakin 2009-05-08 07:38:45 PDT
I filed bug 492037 on the regression.
Comment 15 User image Daniel Veditz [:dveditz] 2009-05-08 10:20:19 PDT
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).
Comment 16 User image Neil Deakin 2009-05-28 09:20:17 PDT
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
Comment 17 User image Samuel Sidler (old account; do not CC) 2009-06-10 14:55:19 PDT
Was this fixed by bug 492037 or did you land this patch on 1.9.0 as well?
Comment 18 User image Al Billings [:abillings] 2009-06-30 17:46:31 PDT
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.
Comment 19 User image Martin Stránský 2009-07-09 06:49:27 PDT
It doesn't seem to affect 1.8.
Comment 20 User image Aakash Desai [:aakashd] 2009-07-21 09:46:00 PDT
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

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