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: 220.127.116.11) Gecko/20080530 Firefox/ 18.104.22.168 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 crashes. Under Ubuntu 8.04, I crash , but no proposal of sending report crashes. Below, links to the first 4 crashes generated by this window. http://crash-stats.mozilla.com/report/index/1d65a463-428a-11dd-a516-001321b13766 http://crash-stats.mozilla.com/report/index/915efac7-428a-11dd-921d-001cc45a2ce4 http://crash-stats.mozilla.com/report/index/915efac7-428a-11dd-921d-001cc45a2ce4 http://crash-stats.mozilla.com/report/index/c5253097-428a-11dd-82e6-001321b13766 Reproducible: Always Steps to Reproduce: 1.Open a new Firefox 3 2.go to http://test03.christophe-charron.org/public/xul/2008_06_21/2008-02-21-test01.php 3.click 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
I crashed using SeaMonkey 2.0a1pre (Gecko 22.214.171.124pre) 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: gklayout!nsXULTemplateQueryProcessorRDF::CheckIsSeparator+0x36 gklayout!nsXULTemplateResultRDF::GetType+0x40 gklayout!nsXULTreeBuilder::IsSeparator+0xca gklayout!nsTreeBodyFrame::PaintRow+0x26c gklayout!nsTreeBodyFrame::PaintTreeBody+0x41d gklayout!PaintTreeBody+0x22 gklayout!nsDisplayGeneric::Paint+0x33 gklayout!nsDisplayList::Paint+0x3e gklayout!nsDisplayWrapList::Paint+0x1e gklayout!nsDisplayClip::Paint+0x58 gklayout!nsDisplayList::Paint+0x3e gklayout!nsLayoutUtils::PaintFrame+0x40d gklayout!PresShell::Paint+0x17d gklayout!nsViewManager::RenderViews+0x9b gklayout!nsViewManager::Refresh+0x307 gklayout!nsViewManager::DispatchEvent+0x3fc gklayout!HandleEvent+0x49 gkwidget!nsWindow::DispatchEvent+0xba gkwidget!nsWindow::DispatchWindowEvent+0x22 gkwidget!nsWindow::OnPaint+0x74a
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] Patch The aforementioned patch. I'm not entirely sure what conflicts this may cause, but this does seem fix the stale rows.
Comment on attachment 328516 [details] [diff] [review] Patch 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.
Created attachment 328614 [details] [diff] [review] Patch v2 Addressing Neil's comments.
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.
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.
Pushed as 17107:78ac7c4b0cf1.
1.9.0 !exploitable report UNKNOWN: Read Access Violation starting at xpcom_core!PL_DHashTableOperate
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.
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.
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.
I filed bug 492037 on the regression.
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 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
Was this fixed by bug 492037 or did you land this patch on 1.9.0 as well?
Verified that this is fixed in 126.96.36.199 using original testcase on website and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:188.8.131.52pre) Gecko/2009063005 GranParadiso/3.0.12pre (.NET CLR 3.5.30729). 184.108.40.206 crashes following test steps.
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 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11pre) Gecko/20090720 Shiretoko/3.5.1pre ID:20090720042942