As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 340796 - [FIX]Crash [@ nsXULDocument::ResumeWalk] during restart (due to last build being different)
: [FIX]Crash [@ nsXULDocument::ResumeWalk] during restart (due to last build be...
Status: RESOLVED FIXED
: crash, fixed1.8.0.5, fixed1.8.1, regression
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: P1 critical with 2 votes (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Neil Deakin
Mentors:
http://bonsai.mozilla.org/cvsblame.cg...
Depends on:
Blocks: 326645
  Show dependency treegraph
 
Reported: 2006-06-08 02:29 PDT by Jesse Ruderman
Modified: 2011-06-09 14:58 PDT (History)
12 users (show)
dveditz: blocking1.8.0.5+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack trace (mac debug) (3.07 KB, text/plain)
2006-06-08 02:30 PDT, Jesse Ruderman
no flags Details
this should do it (91.62 KB, patch)
2006-06-08 07:33 PDT, timeless
neil: superreview-
Details | Diff | Splinter Review
Proposed fix (2.75 KB, patch)
2006-06-12 21:10 PDT, Boris Zbarsky [:bz] (still a bit busy)
jonas: review+
neil: superreview+
jonas: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
1.8 branch patch (2.76 KB, patch)
2006-06-14 20:16 PDT, Boris Zbarsky [:bz] (still a bit busy)
dveditz: approval1.8.0.5+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2006-06-08 02:29:56 PDT
Steps to reproduce, at least on my machine:
1. Launch 2006-06-07 nightly.
2. Close it.
3. Launch trunk build from the command line.

Result: Crash [@ nsXULDocument::ResumeWalk] after "WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Users/admin/trunk/mozilla/xpcom/base/nsExceptionService.cpp, line 191".
Comment 1 User image Jesse Ruderman 2006-06-08 02:30:39 PDT
Created attachment 224841 [details]
stack trace (mac debug)
Comment 2 User image Jesse Ruderman 2006-06-08 02:31:00 PDT
> 3. Launch trunk build from the command line.

I meant "Launch trunk *debug* build from the command line".
Comment 3 User image Jesse Ruderman 2006-06-08 06:32:52 PDT
This happens with today's nightly too, so it's not limited to debug builds or due to changes I have in my tree.
Comment 4 User image timeless 2006-06-08 07:33:45 PDT
Created attachment 224859 [details] [diff] [review]
this should do it
Comment 5 User image neil@parkwaycc.co.uk 2006-06-08 15:57:55 PDT
Comment on attachment 224859 [details] [diff] [review]
this should do it

I want the title to be set even if no attribute could be found. And if you're not going to do it, I want something that will remind people to find out why this is happening. And finally, I want to see peer review.
Comment 6 User image neil@parkwaycc.co.uk 2006-06-10 16:02:06 PDT
OK, so seamonkey -mail -compose -addressbook triggers this presumably because they all try to load the new account wizard into the same DOM window.
Comment 7 User image timeless 2006-06-12 01:52:10 PDT
neil: err, aren't you peer for xul? if not, who is?

note that i didn't claim ownership of this bug. my product doesn't even build xul, i have no affiliation to this bug or any others like it, this patch was posted as a one off.
Comment 8 User image Josh Aas 2006-06-12 12:56:42 PDT
mRootContent is null when we crash. Someone is probably calling ResumeWalk after calling Destroy().

Ben? Bryner? Neil?
Comment 9 User image Josh Aas 2006-06-12 12:57:34 PDT
bz suggested that this bug is fallout for bug 326645, someone might want to look into that
Comment 10 User image timeless 2006-06-12 14:44:11 PDT
that makes sense as an explanation but afaik is entirely unhelpful about dealing with it. unless jesse wants to get a stack trace for destroy and post it here.
Comment 11 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-12 17:33:26 PDT
Well.  Dealing with stuff should be straightforward -- any code that assumes mRootContent is non-null is on crack and needs spanking.  I'll post a patch to that effect later tonight, probably.

What I want to know is whether it's considered a bug that ResumeWalk is being called after Destroy.... I'm not sure it is, but I'm not too clear on what the heck ResumeWalk really is, so...
Comment 12 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-12 20:48:01 PDT
OK, with the steps in comment 6 and a vanilla profile, I can reproduce.  That also fires the following asserts (some of which look bogus, and some of which indicate major XUL bugs, imo):

###!!! ASSERTION: no script global object: 'mScriptGlobalObject != nsnull', file ../../../../../mozilla/content/xul/document/src/nsXULDocument.cpp, line 3186

(due to trying to ExecuteScript off an OnStreamComplete in a document that's already had its script global nulled out...  I wonder why that channel didn't get canceled via the loadgroup...)

###!!! ASSERTION: element has no document: 'doc', file ../../../../../mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp, line 342

(because we're hooking up template builders after we've nuked our DOM)
Comment 13 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-12 21:06:15 PDT
Oh, and also:

###!!! ASSERTION: Event listener added to outer window!: 'win->IsInnerWindow()', file ../../../../mozilla/content/events/src/nsEventListenerManager.cpp, line 1229

because GetScriptGlobalObject will return an outer window after teardown...

We should probably get bugs filed on all of these.
Comment 14 User image Daniel Veditz [:dveditz] 2006-06-12 21:07:22 PDT
Nominating for 1.8.0.5 because this appears to be a regression from a fix we want (bug 326645)
Comment 15 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-12 21:10:21 PDT
Created attachment 225381 [details] [diff] [review]
Proposed fix

I think this is the safe way to go.  Behavior is pretty wild with this change for the testcase in comment 6 (e.g. the account wizard is a 0x0 window), but it's better than crashing...
Comment 16 User image neil@parkwaycc.co.uk 2006-06-13 01:47:26 PDT
Comment on attachment 225381 [details] [diff] [review]
Proposed fix

>+            mRootContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::title,
>+                                  title);
Anything wrong with an 80-character line? ;-)
Comment 17 User image timeless 2006-06-13 03:44:53 PDT
Comment on attachment 225381 [details] [diff] [review]
Proposed fix

>@@ -3499,16 +3503,20 @@ nsXULDocument::OverlayForwardReference::
>     if (id.IsEmpty()) {

I don't think this comment matches the code that follows:
>         // overlay had no id, use the root element
>+        if (!mDocument->mRootContent) {
>+            return eResolve_Error;
Comment 18 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-13 07:35:05 PDT
Sure it matches.  No id, so use the root element as the insertion point.  Then duplicate the other insertion point logic -- no point means bail, otherwise insert.
Comment 19 User image Daniel Veditz [:dveditz] 2006-06-13 14:37:41 PDT
Please land this on trunk/1.8 branches soon for verification
Comment 20 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-13 14:49:16 PDT
I will once trunk reopens.  If that doesn't happen by tomorrow, I'll need someone to land this for me if it's going to make 1.8.0.5...
Comment 21 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-14 20:16:59 PDT
Created attachment 225666 [details] [diff] [review]
1.8 branch patch
Comment 22 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-14 20:18:10 PDT
Fixed on trunk and 1.8 branch.
Comment 23 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-14 20:18:34 PDT
Comment on attachment 225666 [details] [diff] [review]
1.8 branch patch

This fixes a crash regrssion from a security fix we want on branch by adding needed null-checks.
Comment 24 User image Daniel Veditz [:dveditz] 2006-06-15 14:14:32 PDT
Comment on attachment 225666 [details] [diff] [review]
1.8 branch patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 25 User image Boris Zbarsky [:bz] (still a bit busy) 2006-06-19 17:46:54 PDT
Fixed on 1.8.0 branch.

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