37434 - leaking nsParser, HTMLContentSink

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P1)

Description

[jrohlf]

http://www.netscape.com creates 2 nsParsers, one for the initial fetch
then another for the fetch triggered by the javascript:

location.href="http://home.netscape.com/index1.html";

The first nsParser is never freed. nsParser::OnStopRequest() is called
but HTMLContentSink::DidBuildModel() is never called so the circular 
reference between parser/sink is never broken.


This was using a build from roughly 4/22.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Marking confirmed.  We do leak these things on http://www.netscape.com/

Comment 2

[rickg]

One for harish; the cause of this is likely in the docshell/docloader.

Comment 3

[harishd]

Note: I don't see the leak if I load it ( netscape.com -- saved ) from my local 
drive. 

Comment 4

[rusty.lynch]

I was looking into this and came across some more strange behaviour.  I found 
that when I directly loaded http://home.netscape.com/index1.html I would leak a 
parser and html content sink on one of my profiles, but not on another.  

It ended up that the controlling variable was auto-detection of charsets.  If i 
turn on auto-detection of 'Chinese', 'Korean', 'Simplified Chinese', 
or 'Traditional Chinese', then I leak a parser/html sink.  (This is in addition 
to the parser/html sink lost by viewing http://www.netscape.com and then being 
redirected.

The strange thing is that I am having a hell of a time narrowing down a simple 
test case from the huge index1.html.  I haven't been able to reproduce the leak 
on any other pages, and doing basic edits of index1.html (like adding a space 
between lines of text in some locations) will cause the leak to go away.  You 
can see what I have narrowed the page down to at 
http://www.linux86.com/testcases/parser_leak.html . (Just be sure to turn on 
charset auto-detection before visiting the page.)

I'm not sure if this is rooting from the same problem as the redirection leak 
first noted, but if you want to just track down the leak caused by the test 
case in the first comment, be sure to turn off charset auto-detection.

Comment 5

[harishd]

Nominating for beta3.

Comment 6

[Nisheeth Ranjan]

This is a major site and and the parser and content sink are relatively large
leaks.  Marking nsbeta3+.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

My patch on bug 48030 is an attempt to fix this, but it causes
http://www.netscape.com/ to crash.  First, I see an assertion that I didn't see
before:
###!!! ASSERTION: Could not convert external JavaScript to Unicode!:
'NS_SUCCEEDED(rv)', file
/home/david/mozilla/src/mozilla/layout/html/document/src/nsHTMLContentSink.cpp,
line 4624
###!!! Break: at file
/home/david/mozilla/src/mozilla/layout/html/document/src/nsHTMLContentSink.cpp,
line 4624

This is caused by the failure of nsCharsetConverterManager::GetUnicodeDecoder
because the call to do_CreateInstance here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/intl/uconv/src/nsCharsetConverterManager.cpp&rev=1.67&mark=523#515
fails.

Then I crash with the stack trace:

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

#0  0x40126d81 in nsQueryInterface::operator() (this=0xbfffef3c, 
    aIID=@0x41962340, answer=0xbfffee68)
    at /home/david/mozilla/src/mozilla/xpcom/base/nsCOMPtr.cpp:32
#1  0x417e485c in nsCOMPtr<nsIDOMNode>::assign_from_helper (this=0xbfffef48, 
    helper=@0xbfffef3c, aIID=@0x41962340)
    at ../../../dist/include/nsCOMPtr.h:856
#2  0x417e5ad3 in nsCOMPtr<nsIDOMNode>::operator= (this=0xbfffef48, 
    rhs=@0xbfffef3c) at ../../../dist/include/nsCOMPtr.h:614
#3  0x4178b53b in nsDOMSelection::LookUpSelection (this=0x901f450, 
    aContent=0x1, aContentOffset=1100761632, aContentLength=145487576, 
    aReturnDetails=0x905b9a8, aType=1, aSlowCheck=0)
    at /home/david/mozilla/src/mozilla/layout/base/src/nsSelection.cpp:3758
#4  0x417841cf in nsSelection::LookUpSelection (this=0x8f703d8, aContent=0x1, 
    aContentOffset=1100761632, aContentLength=145487576, 
    aReturnDetails=0x905b9a8, aSlowCheck=0)
    at /home/david/mozilla/src/mozilla/layout/base/src/nsSelection.cpp:1780
#5  0x415a2893 in HTMLContentSink::ResumeParsing (this=0x905b9a8)
    at /home/david/mozilla/src/mozilla/layout/html/document/src/nsHTMLContentSin
    k.cpp:4454
#6  0x415a39ba in HTMLContentSink::OnStreamComplete (this=0x905b9a8, 
    aLoader=0x8f62f78, aContext=0x0, aStatus=0, stringLen=1453, 
    string=0x8fdd018 "var WinHeight;\nfunction Pup() {\n  if
((parseFloat(navigator.appVersion)>=5)&&((navigator.appName==\"Netscape\")||(navigator.appName==\"Mozilla\")))\n 
{\n    WinHeight='370';\n  }\n  else\n  {\n    WinHeight='3"...)
    at /home/david/mozilla/src/mozilla/layout/html/document/src/nsHTMLContentSin
    k.cpp:4651

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, it's just a corrupt stack, since HTMLContentSink::ResumeParsing calls into
a parser that no longer exists.  I know how to fix up the patch - it's just a
bit of a pain.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, this is pretty messy.  The document and the parser have been destroyed
when the content sink receives an OnStreamComplete notification.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: changes I was working with... (shows crash more clearly...)

Comment 12

[harishd]

Setting priority to P1.

Comment 13

[leger]

We would like to get this in ASAP if possible.  We may need to downgrade if not 
fixed soon.  But for now...PDT agrees P1

Comment 14

[(not reading, please use [e-mail] instead)]

hoping this fix will also fix bug #49115

Comment 15

[leger]

Putting [pdtp1] in whiteboard

Comment 16

[harishd]

Rick Potts, is coming up with a fix in the docshell/dom code.

Here is my reduced testcase:

<HTML>
 <HEAD>
  <SCRIPT TYPE="text/javascript" LANGUAGE="JavaScript">
   <!--
      location.href="netscp_test.html";
    // -->
  </SCRIPT>
  <script src=netscp.js></script>
 </HEAD>
</HTML>

please create netscp_test.html and netscp.js. Thanx

Comment 17

[rpotts (gone)]

I've just checked in a fix for this bug.

Basically, I added an attribute called StopActiveDocument to the 
nsIDocShellLoadInfo that indicates whether the active document should be stopped 
immediately when the new URI is loaded.
(Normally, the active document is stopped when the data for the new URI starts 
to arrive...)

I added calls to StopActiveDocument before calling nsDocShell::LoadURI(...) in 
nsLocation.cpp

This ensures that when location.href is processed, the current document is 
stopped immediately...

-- rick

Comment 18

[bsharma]

updated qa contact.

Comment 19

[bsharma]

Verified on:
build: 2001-03-29-09-Mtrunk
Platform: Win NT

Marked verified as per the developer comments, since cannot
verify parser leak.