Crash [@ nsGlobalWindow::GetLocalStorage] getting localStorage from removed frame

RESOLVED FIXED in mozilla6

Status

()

--
critical
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jruderman, Assigned: Ms2ger)

Tracking

(Blocks: 2 bugs, {crash, regression, testcase})

Trunk
mozilla6
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(6 attachments)

(Reporter)

Description

8 years ago
Created attachment 522774 [details]
testcase (crashes Firefox when loaded)

First seen 2011-03-28 1:40pm.  Might be a regression from the cedar merge :(
(Reporter)

Comment 1

8 years ago
Created attachment 522776 [details]
stack trace
(Reporter)

Comment 2

8 years ago
Oh I bet it's a regression from bug 639849.
Assignee: nobody → jruderman
Blocks: 639849
Hmm.  Possible if someone nulls out only one of mDoc and mDocument.  Ms2ger, can you look?
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?
(Reporter)

Updated

8 years ago
Assignee: jruderman → Ms2ger
(Reporter)

Comment 5

8 years ago
I must have accidentally clicked "Take" and not noticed that it made a change (rather than simply making a new field visible).
(Assignee)

Comment 6

8 years ago
Created attachment 522812 [details] [diff] [review]
Patch v1: reinstate null checks.

In that case, this should fix it. Jesse, could you test?
Attachment #522812 - Flags: review?(bzbarsky)
Comment on attachment 522812 [details] [diff] [review]
Patch v1: reinstate null checks.

r=me
Attachment #522812 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 8

8 years ago
Created attachment 522941 [details] [diff] [review]
Patch for checkin.
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 9

8 years ago
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
Keywords: checkin-needed
(Assignee)

Comment 10

8 years ago
Created attachment 523768 [details] [diff] [review]
Annotated assertion.

Unless someone has a better idea, I'll just have to annotate the test.
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 11

8 years ago
(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.
Keywords: checkin-needed
(Reporter)

Comment 12

8 years ago
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 ;)
I think we should check this in ... the XPConnect assertion happens quite often on many different things including session restore.

Comment 14

8 years ago
(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.
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.
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

8 years ago
(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.
(Assignee)

Updated

8 years ago
Depends on: 651842
(Assignee)

Comment 19

8 years ago
http://hg.mozilla.org/mozilla-central/rev/0f2d31305fca
Status: NEW → RESOLVED
Last Resolved: 8 years ago
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Looks like the assertion went away with today's tracemonkey merge (the current unexpected-pass orange).
Crash Signature: [@ nsGlobalWindow::GetLocalStorage]
Still crashes in Fennec5.0 beta.
Flags: in-testsuite+
Duplicate of this bug: 670576
Duplicate of this bug: 659904

Comment 24

8 years ago
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
Which fuzzer?

Comment 26

8 years ago
a private one ;)
You need to log in before you can comment on or make changes to this bug.