Closed Bug 417699 Opened 17 years ago Closed 17 years ago

Crash when start SeaMonkey MailNews and Thunderbird [@ nsXULTreeBuilder::SetTree ]

Categories

(MailNews Core :: Backend, defect)

x86
All
defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: tobias, Assigned: smaug)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(7 files, 1 obsolete file)

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.9b4pre) Gecko/2008021423 Mnenhy/0.7.5.20005 SeaMonkey/2.0a1pre SeaMonkey Tinderbox-Builds and also Thunderbird Tinderbox-Builds crash at startup, SM when starting MailNews, even from Browser or with argument -mail. This regressed between 2008-02-14 12:30:00 and tested SM-Tinderbox-Build 2008021423 and later TB 2008021502 Build. Because of limited Checkins for MailNews I suspect one of Neils Checkins, might be the one from 12:34 for Bug 258018 or the other at 16:33 for Bug 415601 so I cc' someone. Well, think this was a Blocker, but feel free to bring it down to major or so.
Keywords: crash, regression
Add some Crash-Reports from 2008021501-regular-Nightly-Build: 98e088ae-dbc7-11dc-9ba8-001a4bd43e5c 9d3ac18a-dbc7-11dc-857a-001a4bd43e5c SeaMonkey is crashing in "nsXULTreeBuilder::SetTree(nsITreeBoxObject*" but I don't know how to add the relevant Part of the Stack here from Crash-Reporter Site, so only the IDs and one Link: http://crash-stats.mozilla.com/report/index/98e088ae-dbc7-11dc-9ba8-001a4bd43e5c
Summary: Crash when start SeaMonkey MailNews and Thunderbird → Crash when start SeaMonkey MailNews and Thunderbird [@ nsXULTreeBuilder::SetTree ]
Fortunately nothing in your stacks have any relevance to nsUInt32Array. Naturally my own debug builds don't have any problems...
(In reply to comment #2) > Fortunately nothing in your stacks have any relevance to nsUInt32Array. Yum, thats right. Have to take a new look in Bonsai, sorry for my fast suspicion. > > Naturally my own debug builds don't have any problems... > Nice, also I have heard, that someone using WinNT 6.0 was able to run MailNews too from 2008021423-Tinderbox-Build without crashing. Regression Range was between 2008-02-14 12:30 and 2008-02-14 23:31: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-14+12%3A30%3A00&maxdate=2008-02-14+23%3A31%3A00&cvsroot=%2Fcvsroot
Also applies to todays linux build, crashes on startup. For me running in -safe-mode works though. Tracked it down to lightning (trunk version, had a version from a week back, updated to current build but no help for the crash.)
Blocks: 409111
(In reply to comment #5) > After backing out https://bugzilla.mozilla.org/attachment.cgi?id=303133 of Bug ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 409111 the crash is gone. Should be https://bugzilla.mozilla.org/attachment.cgi?id=303297
I'm looking at this.
Seems like the box object change revealed some null pointer crash in template handling. The testcase shows that similar problem has been there for a long time.
Enn, I think most of the cases mRoot is used should check whether it is null.
Looks like just a null-check in nsTreeBuilder::SetTree is needed. There's already one in Rebuild.
Yes, that fixes this bug, I hope (just compiling TB), but mRoot is used elsewhere too. See the testcase, for example.
I can't reproduce this bug locally using Thunderbird :/ But I'll upload a patch, if someone could then test it.
Attached patch add null checks (obsolete) (deleted) — Splinter Review
Anyone willing to try this?
> NS_IMETHODIMP > nsXULTemplateBuilder::GetRoot(nsIDOMElement** aResult) > { >+ NS_ENSURE_TRUE(mRoot, NS_ERROR_NOT_INITIALIZED); > return CallQueryInterface(mRoot, aResult); GetRoot should just return null, not cause an error.
Attached patch .root doesn't throw (deleted) — Splinter Review
Attachment #303551 - Attachment is obsolete: true
Attached image Result of applying 303553 (deleted) —
I have applied attachment 303553 [details] [diff] [review] to a new build of SM 2.0 on linux. No crash, but look at the resulting MailNews-Window. *g* Scaled down to 800x600
Attached patch Clear mView (deleted) — Splinter Review
Does this make any difference? (Would be so great to be able to reproduce this.)
Attached image Result of applying 303567 (deleted) —
Seems to be OK now.
Great. I'm now compiling Seamonkey, just to see if I can reproduce the problem there. Attachment 303567 [details] [diff] is the right thing to do, IMO. Is TB or SM moving xul:tree elements in the DOM? So first initializing them in one place and then moving to a new place. That could lead to the problem, I think
We can do, although not in the default configuration that Hartmut seems to have.
Can't reproduce on seamonkey either. And I tried to write some testcases, which remove and add xul:tree back, but those work with or without the patch. Same thing when setting .style.display to 'none' and back.
I don't like this too much, but should bring back the old behavior and same behavior as what 1.8 has. Could anyone test this?
Attached file .mozconfig of Hartmut (deleted) —
Mhm, i don't think, my .mozconfig is that unusual. But here it is.
(In reply to comment #23) > Could anyone test this? Done. Works. No crash.
I had the Crash on TB (checkout start: Feb 07 18:11:52 PST 2008). After installing attachment (id=303533) I had the same view as Hartmut. At this point I tried 'save-mode' (OK I should have done this before) and the view looks fine. The only Add-on that was installed in my Test-Profile was 'Lightning'. So I deactivate it and the view was fine too! To check this I got back to the original version: No crash! Activating 'Lightning': Crash. Finally I applied attachment (id=303567) and the crash was gone. HTH
Comment on attachment 303586 [details] [diff] [review] v3, clear boxObject when unbinding Ok, let's take this then. Least-regression-risky. Adds back those 2 ClearBoxObjectFor calls that were removed in bug 409111. (Similar are there in 1.8). Also adds null checks to template handling to prevent crashes. Doesn't regress bug 409111 because creating boxobjects is still possible for elements which aren't in a document. A testcase for this bug would be great.
Attachment #303586 - Flags: superreview?(jonas)
Attachment #303586 - Flags: review?(jonas)
(In reply to comment #26) > I had the Crash on TB (checkout start: Feb 07 18:11:52 PST 2008). Sorry - That must be (checkout start: Feb 15 10:11:52 PST 2008)
(In reply to comment #27) > A testcase for this bug would be great. Well, the comment of Alfred showed me, that i should have tested more. At the moment i'm using an SM, which formerly crashed. This is possible, because i disabled Mnenhy. But SM withou Mnenhy, eeh, your patch is very welcome. ;)
I meant a minimal testcase.
Attachment #303586 - Flags: superreview?(jonas)
Attachment #303586 - Flags: superreview+
Attachment #303586 - Flags: review?(jonas)
Attachment #303586 - Flags: review+
Comment on attachment 303586 [details] [diff] [review] v3, clear boxObject when unbinding Asking approval to get the regression fixed.
Attachment #303586 - Flags: approval1.9?
Does this also fix an XPConnect assertion I've been seeing lately? xpc3250!DEBUG_CheckForComponentsInScope+0x70 xpc3250!XPCWrappedNativeScope::FindInJSObjectScope+0x9e xpc3250!GetContextFromObject+0x87 xpc3250!nsXPCWrappedJSClass::CallMethod+0x5c xpc3250!nsXPCWrappedJS::CallMethod+0x3f xpcom_core!PrepareAndDispatch+0x314 xpcom_core!SharedStub+0x16 gklayout!nsTreeBoxObject::Clear+0x61 gklayout!nsDocument::ClearBoxObjectFor+0x3d gklayout!nsNodeUtils::LastRelease+0x21c gklayout!nsGenericElement::Release+0xc8 gklayout!nsXULElement::Release+0xd xpc3250!XPCJSRuntime::GCCallback+0x547 gklayout!DOMGCCallback+0x1d js3250!js_GC+0xdf3 js3250!JS_GC+0x48 xul!nsXREDirProvider::DoShutdown+0x154 xul!ScopedXPCOMStartup::~ScopedXPCOMStartup+0x5b xul!XRE_main+0x321d seamonkey!NS_internal_main+0x10c seamonkey!wmain+0x101 seamonkey!wmainCRTStartup+0x12c kernel32!BaseProcessStart+0x23
(In reply to comment #30) > I meant a minimal testcase. > Humm, sorry that I was not able to prepare a minimal testcase too. I have noticed, that the crash only occours when Mnenhy (testbuild, you have to ask mnyromyr@tprac.de to get this) with activated "MailNews-Sidebar", or Lightning was installed, so I bring the Bug down to "critical" for now.
Severity: blocker → critical
Comment on attachment 303586 [details] [diff] [review] v3, clear boxObject when unbinding a=mconnor on behalf of 1.9 drivers
Attachment #303586 - Flags: approval1.9? → approval1.9+
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Verified=Fixed Have tested with Windows SeaMonkey Tinderbox-Build 2008021900 with Mnenhy installed and MailNews-Sidebar activated and Windows Thunderbird Tinderbox-Build 2008021902 with Lightning installed. No crash. THX.
Status: RESOLVED → VERIFIED
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee: nobody → Olli.Pettay
Status: REOPENED → NEW
Status: NEW → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Blocks: 417673
Depends on: 433429
Product: Core → MailNews Core
Crash Signature: [@ nsXULTreeBuilder::SetTree ]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: