367310 - immediately after closing history crash opening unvisited links [@ nsTreeRows::FindByResource]

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Felix Miata]

How I reproduce:
1-open some pages in tabs
2-open CZ
3-open history
4-open some tabs from context menu in history window
5-close history window
6-open some links from context menu in new tabs

This has also apparently happened from a form submit instead of context menu after history window close.

I was told several months ago on moznet that the crash signature from this matched that of bug 330776, so in https://bugzilla.mozilla.org/show_bug.cgi?id=330776#c5 on 2 Sept. and later comments I attached the talkback IDs. Yesterday I built from a fresh pull after applying the patch from that bug. The result was the crashes didn't stop. Talkbacks currently showing from SM trunk builds are TB28382395K TB28382631G TB28382942H TB28460074Z TB28460199M. BT from GDB using my build:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1086294848 (LWP 9517)]
0x415ac59e in nsTreeRows::FindByResource (this=0xb5d1890, aResource=0x81e2388)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsTreeRows.cpp:189
189                 rv = iter->mMatch->mResult->GetResource(getter_AddRefs(findres));
(gdb) bt
#0  0x415ac59e in nsTreeRows::FindByResource (this=0xb5d1890, aResource=0x81e2388)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsTreeRows.cpp:189
#1  0x415bc406 in nsXULTreeBuilder::HasGeneratedContent (this=0xb5d1800, aResource=0x81e2388,
    aTag=0x0, aGenerated=0xbfe91c7c)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsXULTreeBuilder.cpp:1084
#2  0x415aaf52 in nsContentTestNode::Constrain (this=0xb6b24f0, aInstantiations=@0xbde1fd8)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsContentTestNode.cpp:114
#3  0x415b265c in TestNode::Constrain (this=0xb6a3bf0, aInstantiations=@0xbde1fd8)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsRuleNetwork.cpp:410
#4  0x415c8a63 in nsXULTemplateQueryProcessorRDF::Propagate (this=0xb5e24e8, aSource=0x81e2388,
    aProperty=0x8170658, aTarget=0x85aa4f8)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsXULTemplateQueryProcessorRDF.cpp:814
#5  0x415cca5a in nsXULTemplateQueryProcessorRDF::OnAssert (this=0xb5e24e8, aDataSource=0xb5ccb38,
    aSource=0x81e2388, aProperty=0x8170658, aTarget=0x85aa4f8)
    at /usr/local/devel/mozilla/content/xul/templates/src/nsXULTemplateQueryProcessorRDF.cpp:643
#6  0x417faf82 in CompositeDataSourceImpl::OnAssert (this=0xb5ccb38, aDataSource=0x85a1ed8,
    aSource=0x81e2388, aProperty=0x8170658, aTarget=0x85aa4f8)
    at /usr/local/devel/mozilla/rdf/base/src/nsCompositeDataSource.cpp:1462
#7  0x4185bea4 in nsGlobalHistory::NotifyAssert (this=0x85a1ec8, aSource=0x81e2388,
    aProperty=0x8170658, aValue=0x85aa4f8)
    at /usr/local/devel/mozilla/xpfe/components/history/src/nsGlobalHistory.cpp:3148
#8  0x4185c1d2 in nsGlobalHistory::NotifyFindAssertions (this=0x85a1ec8, aSource=0xbde53e0,
    aRow=0x8170658)
    at /usr/local/devel/mozilla/xpfe/components/history/src/nsGlobalHistory.cpp:3532
#9  0x41860d64 in nsGlobalHistory::AddURI (this=0x85a1ec8, aURI=0xc024b00, aRedirect=0,
    aTopLevel=1, aReferrer=0xbfd4f70)
    at /usr/local/devel/mozilla/xpfe/components/history/src/nsGlobalHistory.cpp:696
#10 0x41be2f78 in nsDocShell::AddToGlobalHistory (this=0xb7173c8, aURI=0xc024b00, aRedirect=0,
    aChannel=0xc023cac) at /usr/local/devel/mozilla/docshell/base/nsDocShell.cpp:8165
#11 0x41be34a3 in nsDocShell::OnNewURI (this=0xb7173c8, aURI=0xc024b00, aChannel=0xc023cac,
    aLoadType=2097153, aFireOnLocationChange=0, aAddToGlobalHistory=1)
---Type <return> to continue, or q <return> to quit---q

Comment 1

[Felix Miata]

First build tested that crashes: Linux SM trunk 2006081701
Last build tested that does not crash: Linux SM trunk 2006081601
SUSE Linux 10.0 with libstdc++-4.0.2_20050901-3
history.dat filesize 750k (220 days)
SUSE Linux 10.2 with libstdc++41-4.1.2_20061115-5
history.dat filesize 6k (20 days)

To reproduce:
1-open browser into automatic default homepage
2-open history
3-set sort to title
4-open twisty for today
5-open twisty for some site
6-select some page that has links to open with double left click
7-switch back to history window
8-set sort to last visited
9-close twisty on the site
10-close twisty on the day
11-X (close) history window
12-from context menu open some unvisited link in new tab
13-watch talkback window open following the crash

Comment 2

[Andrew Schultz]

simpler steps to reproduce:
1. Open SeaMonkey browser
2. Open History (Go->History)
3. Close History
4. Click on a link not already in history [maybe crash]
[repeat steps 2-4]

SeaMonkey often crashes on the first pass and rarely makes it more than 3 times through.  It is not necessary to enable accessibility.

In gdb, iter->mMatch->mResult is 0xd8d8d8d8 as in bug 330776.  The patch from bug 330776 does not prevent the crash for this bug (does it actually fix 330776?)  I originally thought these could be separate bugs, but as I look at it more, it seems more and more likely that this is the same as bug 330776.

By pulling CVS by date, I was able to determine that this started crashing after bug 285631 landed.

Comment 3

[Neil Deakin]

Does it crash if you add the following to nsXULTreeBuilder.cpp:

NS_IMETHODIMP
nsXULTreeBuilder::SetTree(nsITreeBoxObject* aTree)
{
    NS_PRECONDITION(mRoot, "not initialized");

    mBoxObject = aTree;

    // If this is teardown time, then we're done.
-    if (! mBoxObject)
+    if (! mBoxObject) {
+        Uninit(PR_FALSE;
         return NS_OK;
+    }

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

-    if (! mBoxObject)
+    if (!mBoxObject) {
+        Uninit(PR_FALSE);
         return NS_OK;
+    }
Does seem to help.
Without it I can easily reproduce the crash using the steps mentioned in
comment #2, but with that change, couldn't get any crashes.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Uninit(PR_FALSE)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: handles re-calling setTree(...)

Comment 7

[Neil Deakin]

Comment on attachment 252931 [details] [diff] [review]
handles re-calling setTree(...)

Is there some reason why this has an advantage over calling Uninit?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 252931 [details] [diff] [review]
handles re-calling setTree(...)

ah, idiot me. Uninit doesn't set mQueryProcessor to null, as I somehow misread/misremembered.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 252577 [details] [diff] [review]
Uninit(PR_FALSE)

Since this is actually your patch, I could say r+ :)

Comment 10

[Neil Deakin]

Olli, did you check this in?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsXULTreeBuilder.cpp#759
Doesn't look like so :)

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Checked in