Last Comment Bug 646184 - Crash [@ nsGlobalWindow::GetLocalStorage] getting localStorage from removed frame
: Crash [@ nsGlobalWindow::GetLocalStorage] getting localStorage from removed f...
Status: RESOLVED FIXED
: crash, regression, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla6
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
Mentors:
: 659904 670576 (view as bug list)
Depends on: 651842
Blocks: 326633 594645 639849
  Show dependency treegraph
 
Reported: 2011-03-29 13:06 PDT by Jesse Ruderman
Modified: 2011-08-12 09:08 PDT (History)
15 users (show)
martijn.martijn: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (crashes Firefox when loaded) (281 bytes, text/html)
2011-03-29 13:06 PDT, Jesse Ruderman
no flags Details
stack trace (1.33 KB, text/plain)
2011-03-29 13:08 PDT, Jesse Ruderman
no flags Details
Patch v1: reinstate null checks. (1.74 KB, patch)
2011-03-29 14:42 PDT, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Patch for checkin. (2.61 KB, patch)
2011-03-29 23:59 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Annotated assertion. (2.62 KB, patch)
2011-04-02 02:40 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
details from log in comment 9 (11.05 KB, text/plain; charset=UTF-8)
2011-04-17 15:21 PDT, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
no flags Details

Description Jesse Ruderman 2011-03-29 13:06:29 PDT
Created attachment 522774 [details]
testcase (crashes Firefox when loaded)

First seen 2011-03-28 1:40pm.  Might be a regression from the cedar merge :(
Comment 1 Jesse Ruderman 2011-03-29 13:08:51 PDT
Created attachment 522776 [details]
stack trace
Comment 2 Jesse Ruderman 2011-03-29 13:11:19 PDT
Oh I bet it's a regression from bug 639849.
Comment 3 Boris Zbarsky [:bz] 2011-03-29 13:17:14 PDT
Hmm.  Possible if someone nulls out only one of mDoc and mDocument.  Ms2ger, can you look?
Comment 4 Boris Zbarsky [:bz] 2011-03-29 13:17:59 PDT
Oh, wait.  They're probably just both null and the code lost the null-check.

Jesse, are you patching?  If not, please reassign to Ms2ger?
Comment 5 Jesse Ruderman 2011-03-29 14:03:11 PDT
I must have accidentally clicked "Take" and not noticed that it made a change (rather than simply making a new field visible).
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-03-29 14:42:37 PDT
Created attachment 522812 [details] [diff] [review]
Patch v1: reinstate null checks.

In that case, this should fix it. Jesse, could you test?
Comment 7 Boris Zbarsky [:bz] 2011-03-29 20:01:10 PDT
Comment on attachment 522812 [details] [diff] [review]
Patch v1: reinstate null checks.

r=me
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2011-03-29 23:59:09 PDT
Created attachment 522941 [details] [diff] [review]
Patch for checkin.
Comment 9 :Ehsan Akhgari (away Aug 1-5) 2011-04-01 15:23:52 PDT
Boris landed this on cedar, but I had to back it out because of oranges like this:

http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1301693776.1301694328.13610.gz

Original landing: http://hg.mozilla.org/projects/cedar/rev/649f50ed53ca
Backout: http://hg.mozilla.org/projects/cedar/rev/779e2ba9810b
Comment 10 :Ms2ger (⌚ UTC+1/+2) 2011-04-02 02:40:07 PDT
Created attachment 523768 [details] [diff] [review]
Annotated assertion.

Unless someone has a better idea, I'll just have to annotate the test.
Comment 11 :Ehsan Akhgari (away Aug 1-5) 2011-04-02 06:18:50 PDT
(In reply to comment #10)
> Created attachment 523768 [details] [diff] [review]
> Annotated assertion.
> 
> Unless someone has a better idea, I'll just have to annotate the test.

Seems like a bad idea to me.  We should understand the assertion.  CCing some people who might know stuff about xpconnect.
Comment 12 Jesse Ruderman 2011-04-02 11:00:12 PDT
The orange is the new test hitting the assertion

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!  (stack and details follow): 'Error', file /builds/slave/ced-osx64-dbg/build/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 761

dbaron can probably tell us which existing bug report covers it ;)
Comment 13 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-04-17 05:31:45 PDT
I think we should check this in ... the XPConnect assertion happens quite often on many different things including session restore.
Comment 14 :Ehsan Akhgari (away Aug 1-5) 2011-04-17 14:38:12 PDT
(In reply to comment #13)
> I think we should check this in ... the XPConnect assertion happens quite often
> on many different things including session restore.

I would be fine with us landing this with a assertion annotations _if_ we knew that the triggered assertion is an instance of an existing bug.  So far, I've not seen anything which suggests this is true.
Comment 15 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-04-17 14:59:02 PDT
Ehsan and I chatted on IRC a bit about this.  Does the assertion happen before and after the fix with the testcase?  If the assertion is added by the fix, then we need to figure it out.
Comment 16 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-04-17 15:21:07 PDT
Created attachment 526635 [details]
details from log in comment 9
Comment 17 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-04-17 15:26:34 PDT
I think the assertion:
 * is not caused by the patch
 * might or might not have the same underlying cause as bug 631289
 * is not related to any of the existing no-Components assertion bugs
Comment 18 :Ehsan Akhgari (away Aug 1-5) 2011-04-18 18:16:15 PDT
(In reply to comment #17)
> I think the assertion:
>  * is not caused by the patch
>  * might or might not have the same underlying cause as bug 631289
>  * is not related to any of the existing no-Components assertion bugs

In that case, I'd be fine with this patch landing if you file a new bug for the assertion, and add the bug # to the reftest manifest annotating the assertion as a comment.
Comment 19 :Ms2ger (⌚ UTC+1/+2) 2011-04-22 06:31:40 PDT
http://hg.mozilla.org/mozilla-central/rev/0f2d31305fca
Comment 20 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-04-26 12:08:18 PDT
Looks like the assertion went away with today's tracemonkey merge (the current unexpected-pass orange).
Comment 21 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-06-23 07:54:49 PDT
Still crashes in Fennec5.0 beta.
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-12 11:09:54 PDT
*** Bug 670576 has been marked as a duplicate of this bug. ***
Comment 23 Boris Zbarsky [:bz] 2011-07-19 20:32:55 PDT
*** Bug 659904 has been marked as a duplicate of this bug. ***
Comment 24 David Maciejak 2011-07-21 08:11:09 PDT
just stumble upon today with my fuzzer using latest ubuntu linux and firefox 5.0,
here an extract from gdb

Program received signal SIGSEGV, Segmentation fault.
0xb7115955 in nsGlobalWindow::GetLocalStorage (this=0xa19ec0b0, aLocalStorage=0xbfffde4c)
    at /build/buildd/firefox-5.0+build1+nobinonly/build-tree/mozilla/dom/base/nsGlobalWindow.cpp:7987


regards,
david
Comment 25 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-21 09:59:42 PDT
Which fuzzer?
Comment 26 David Maciejak 2011-07-21 10:29:16 PDT
a private one ;)

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