854864 - crash in nsDocShell::InternalLoad when targeting a load to a docshell that's had nsFrameLoader::StartDestroy but hasn't destroyed its docshell yet

Status: RESOLVED WORKSFORME

(Core :: DOM: Navigation, defect)

Description

[Paul Silaghi, QA [:pauly]]

This bug was filed from the Socorro interface and is 
report bp-7117f32c-220c-4220-a01b-d45542130325 .
============================================================= 

Follow-up for bug 820067

Comment 1

[:Benjamin Peterson]

I don't think this anything to do with 820067.

Comment 2

[Scoobidiver (away)]

Stack traces are various. In addition, it's a low volume crash.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsDocShell%3A%3AInternalLoad%28nsIURI*%2C+nsIURI*%2C+nsISupports*%2C+unsigned+int%2C+wchar_t+const*%2C+char+const*%2C+nsAString_internal+const%26%2C+nsIInputStream*%2C+nsIInputStream*%2C+unsigned+int%2C+nsISHEntry*%2C+bool%2C+nsIDocShell**%2C+nsIRequest**%29

Comment 3

[Bob Clary [:bc] (inactive)]

1. https://leboutique.com/?new=1
2. wait for it.

bp-6c8f1fa5-8246-4fa4-8d98-6582c2151214
[@ nsDocShell::InternalLoad ]

bp-f95da4e2-c5aa-47d0-909b-a8eed2151214
[@ nsIContent::IsHTMLElement ]

Bughunter reproduced this on Beta/43, Aurora/44, Nightly/45 on Linux, OSX, Windows.

Debug builds assert:

Assertion failure: mRawPtr != 0 (You can't dereference a NULL nsCOMPtr with operator->().), at c:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\src\obj-firefox\dist\include\nsCOMPtr.h:734

Comment 4

[Bob Clary [:bc] (inactive)]

Attachment: Assertion stack

>Thread 0 (crashed)
> 0  xul.dll!nsCOMPtr<mozilla::dom::Element>::operator->() [nsCOMPtr.h:f07e71078bc8 : 734 + 0x29]
>    eip = 0x5c1a1aad   esp = 0x0030bb50   ebp = 0x0030bfe8   ebx = 0x00000000
>    esi = 0x000002de   edi = 0x0a941d00   eax = 0x5f0ce2f8   ecx = 0x6e410ad9
>    edx = 0x00934d61   efl = 0x00200206
>    Found by: given as instruction pointer in context
> 1  xul.dll!nsDocShell::InternalLoad(nsIURI *,nsIURI *,bool,nsIURI *,unsigned int,nsISupports *,unsigned int,wchar_t const *,char const *,nsAString_internal const &,nsIInputStream *,nsIInputStream *,unsigned int,nsISHEntry *,bool,nsAString_internal const &,nsIDocShell *,nsIURI *,nsIDocShell * *,nsIRequest * *) [nsDocShell.cpp:f07e71078bc8 : 9595 + 0x11]
>    eip = 0x5e0bc012   esp = 0x0030bb58   ebp = 0x0030bfe8
>    Found by: call frame info
> 2  xul.dll!nsDocShell::InternalLoad(nsIURI *,nsIURI *,bool,nsIURI *,unsigned int,nsISupports *,unsigned int,wchar_t const *,char const *,nsAString_internal const &,nsIInputStream *,nsIInputStream *,unsigned int,nsISHEntry *,bool,nsAString_internal const &,nsIDocShell *,nsIURI *,nsIDocShell * *,nsIRequest * *) [nsDocShell.cpp:f07e71078bc8 : 9766 + 0x94]
>    eip = 0x5e0bc76b   esp = 0x0030bff0   ebp = 0x0030c4d0
>    Found by: call frame info
> 3  xul.dll!nsDocShell::OnLinkClickSync(nsIContent *,nsIURI *,wchar_t const *,nsAString_internal const &,nsIInputStream *,nsIInputStream *,nsIDocShell * *,nsIRequest * *) [nsDocShell.cpp:f07e71078bc8 : 13540 + 0xce]
>    eip = 0x5e0c345b   esp = 0x0030c4d8   ebp = 0x0030c910
>    Found by: call frame info
> 4  xul.dll!mozilla::dom::HTMLFormElement::SubmitSubmission(nsFormSubmission *) [HTMLFormElement.cpp:f07e71078bc8 : 830 + 0x8a]
>    eip = 0x5d588690   esp = 0x0030c918   ebp = 0x0030ca1c
>    Found by: call frame info
> 5  xul.dll!mozilla::dom::HTMLFormElement::DoSubmit(mozilla::WidgetEvent *) [HTMLFormElement.cpp:f07e71078bc8 : 683 + 0xa]
>    eip = 0x5d56af28   esp = 0x0030ca24   ebp = 0x0030ca38
>    Found by: call frame info

Comment 5

[Paul Silaghi, QA [:pauly]]

https://crash-stats.mozilla.com/report/index/94b29427-5d1d-4c2f-86e6-191622151214
Crashed while scrolling on https://manuals.info.apple.com/MANUALS/1000/MA1565/en_US/iphone_user_guide.pdf using PDF.js.

Comment 6

[Bob Clary [:bc] (inactive)]

more bughunter crashes on the same site:

https://leboutique.com/catalog/women/handbag?_openstat=ZGlyZWN0LnlhbmRleC5ydTsxMjg4MzEyMTsxMjk1MzQyOTg1O3N2aXQyNC5uZXQ6bmE&discount=85&new=1&utm_campaign=Y_Bags_NET

bp-d3e7a7a6-c697-4894-9881-e65502151226

This crash/assertion are common on https://leboutique.com

Comment 7

[Carsten Book [:Tomcat]]

Attachment: windows debug build stack with todays m-c tip

Comment 8

[Carsten Book [:Tomcat]]

(In reply to Bob Clary [:bc:] from comment #6)
> more bughunter crashes on the same site:
> 
> https://leboutique.com/catalog/women/
> handbag?_openstat=ZGlyZWN0LnlhbmRleC5ydTsxMjg4MzEyMTsxMjk1MzQyOTg1O3N2aXQyNC5
> uZXQ6bmE&discount=85&new=1&utm_campaign=Y_Bags_NET
> 
> bp-d3e7a7a6-c697-4894-9881-e65502151226
> 
> This crash/assertion are common on https://leboutique.com

using this url is causing now Assertion failure: mRawPtr != 0 (You can't dereference a NULL nsCOMPtr with operator->().), at c:\users\mozilla\debug-builds\mozilla-central\firefox-debug\dist\include\nsCOMPtr.h:734

Comment 9

[Nathan Froyd [:froydnj]]

I can reproduce this crash using steps in comment 3.  The JS stack looks like:

0 YottosTracker/this.send() ["https://cdn.yottos.com/getmyad/_t.js":1]
    this = [object Object]
1 YottosTracker/this.aevent(d = 1, b = "add") ["https://cdn.yottos.com/getmyad/_t.js":1]
    this = [object Object]
2 <TOP LEVEL> ["https://leboutique.com/?new=1":1]
3 bi/<() ["https://www.googletagmanager.com/gtm.js?id=GTM-WVXJN6":75]
    this = [object Window]
4 bi/<([object Event]) ["https://www.googletagmanager.com/gtm.js?id=GTM-WVXJN6":76]
    this = [object HTMLScriptElement]

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Minimalish testcase: will crash

This is fairly exciting...

The basic problem is that we're submitting a form and targeting a named frame.  And that named frame is a zombie.

In particular, we're inserting a <script> that does the following:

1)  Removing an <iframe> from the DOM.
2)  Readding that <iframe> to the DOM.
3)  Doing a form submission targeting that iframe's name.

Normally the way this would work is that the iframe removal would tear down various stuff via nsFrameLoader::StartDestroy, post a script runner to Destroy the docshell, then finish that up before returning control to script.  But in this case, by the time we run the scriptrunner, which calls nsDocument::MaybeInitializeFinalizeFrameLoaders, we still have mUpdateNestLevel == 2 because we're still under the update that we started for the insertion of the <script> element.  So we do NOT finalize the frameloader and leave its docshell hanging out.  Then we end up back in script, add the <iframe> back to the DOM (probably creates a _another_ docshell, but the old one is still hanging out in the docshell tree), then do the submit and target the half-dead docshell...  This is the bug, of course.

Olli, any idea how we can make this work slightly better?  :(

Comment 11

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

oh, hmm, EndUpdate decreases mUpdateNestLevel _after_ removing script blocker.

Comment 12

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

decreasing mUpdateNestLevel before removing script blocker seems to fix the crash in minimal testcase, but I need to still debug why :)

Comment 13

[Bob Clary [:bc] (inactive)]

Firefox 46
bp-7617cb83-3f7f-4994-ae4a-95f742160520
[@ nsDocShell::InternalLoad ]

Nightly/49
bp-094bfa49-1580-48a8-9eef-958ed2160520
[@ nsDocShell::InternalLoad ]

Comment 15

[Nicholas Nethercote [inactive]]

I see 40 occurrences of this crash in the wild in the past 7 days, in versions ranging from FF 13 to FF 49.

Comment 16

[Jesse Ruderman]

Same symptoms as bug 443655. Not marking as a dup, because each bug has some analysis, and I'm not sure they're the same bug anyway.

Comment 17

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:

while testing bug 1260669 and new reports i was able to reproduce the crash - https://leboutique.com/catalog/men/clothes?new=1#_=_ instantly crashes the browser nightly to beta opt and debug .

I guess since we still get reports at least from this site we should fix this.

Comment 18

[Marcia Knous [:marcia]]

Tracking- for 52. Pretty low volume of these signatures on the trunk. The signature that has the most crashes, 254 is nsIContent::IsHTMLElement is on 49.0.1.

Comment 19

[Julien Cristau [:jcristau]]

(In reply to Marcia Knous [:marcia - use ni] from comment #18)
> Tracking- for 52. Pretty low volume of these signatures on the trunk. The
> signature that has the most crashes, 254 is nsIContent::IsHTMLElement is on
> 49.0.1.

Likewise for 50 and 51, not tracking.

Comment 20

[Carsten Book [:Tomcat]]

also can reproduce this in https://leboutique.com/catalog/women/dress?new=1

another crash signature that this result in is https://crash-stats.mozilla.com/report/index/823229f4-0783-43cf-ac38-b514e2161103

xul.dll@0x691559 | xul.dll@0x14dde09 | xul.dll@0x14de5ae | xul.dll@0x14e3b54 | xul.dll@0xdb0a90 | xul.dll@0xd96460 | xul.dll@0xd964cb | xul.dll@0xdb0252 | xul.dll@0xc302b7 | xul.dll@0xc95015 | xul.dll@0x1c91ed8 | xul.dll@0x1c91d1c | xul.dll@0x1c971bb

Comment 21

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

This is WFM. (doing some end-of-year clean up to my needinfo queue)
There are other unrelated crashes in InternalLoad