Closed Bug 243338 Opened 21 years ago Closed 19 years ago

parser hangs on particular combination of unmatched tags

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED WORKSFORME

People

(Reporter: danm.moz, Unassigned)

References

Details

(Keywords: hang)

Attachments

(4 files)

aforementioned test as an attachment. warning: hangs the parser!
150 bytes, text/html
Details
A reduced testcase
85 bytes, text/html
Details
CSSLoader changes
4.60 KB, patch
bzbarsky
: review-
bzbarsky
: superreview-
Details | Diff | Splinter Review
original Website
884 bytes, text/html
Details 
From http://www.chip.de/ via Mozillazine:

Firefox 20040510 build. This particular HTML makes it hang:

<html>
<body>
<table>
<head>
<link rel="stylesheet" type="text/css" href="teststyle.css">
</head> <!-- not necessary -->
<p><div></p>
</body>
</html>

I'm not saying it's good HTML. I'm not saying it ever happens in the real world:
in fact it's obviously written expressly to cause Firefox problems. I'm just
saying it hangs the browser. It gets stuck with this stack trace:

CNavDTD::HandleEndToken
CNavDTD::HandleToken
CNavDTD::BuildModel
nsParser::BuildModel
nsParser::ResumeParse
nsParser::ContinueParsing
nsParser::HandleParserContinueEvent
nsParserContinueEvent::HandleEvent
PL_HandleEvent

Within CNavDTD::HandleEndToken, it hits this bit over and over and over:

if(gHTMLElements[theChildTag].HasSpecialProperty(kHandleStrayTag) &&
    mDTDMode != eDTDMode_full_standards &&
    mDTDMode != eDTDMode_almost_standards) {
  // Oh boy!! we found a "stray" tag. Nav4.x and IE introduce line break in
  // such cases. So, let's simulate that effect for compatibility.
  // Ex. <html><body>Hello</P>There</body></html>
  PRBool theParentContains=-1; //set to -1 to force canomit to recompute.
  if(!CanOmit(theParentTag,theChildTag,theParentContains)) {
    IF_HOLD(aToken);
    mTokenizer->PushTokenFront(aToken); //put this end token back...
    CHTMLToken* theToken = NS_STATIC_CAST(CHTMLToken*,
                mTokenAllocator->CreateTokenOfType(eToken_start,theChildTag));
    mTokenizer->PushTokenFront(theToken); //put this new token onto stack...
  }
}
return result;

  

  



    
    

    
  

      
      
      
      

        Attached file
          
        A reduced testcase
        — 
      

    


  
New attachment with reduced testcase added

  

  



        Attachment #148234 -
        Attachment description: A testcase, wich will hang Mozilla and crash the system because of low memory → A reduced testcase

    
  
Keywords: hang
OS: Windows XP → All
Hardware: PC → All
Flags: blocking1.7?

    
    

    
  
This is very odd... nightlies seem to indicate that this regressed between
2004-05-06-08 and 2004-05-07-08, fingering bug 242503.  But my local build with
that fix (even with the fix for bug 243064 backed out) does _not_ hang...  Note
also that this build was pulled at "Sat May  8 00:29:04 CDT 2004".

Although... is the <link> necessary?  If so, this build is not a good one to
test in -- it has all sorts of fun stylesheet loading changes in it.  ;)

Dan, if you have a build showing this, could you try backing out the bug 242503
patch in it?

  

  



    
    

    
  
Yup! Reverting to 3.429 fixes this bug. That changes things.

  

  


Keywords: regression

    
    

    
  
Abdulkadir reports this takes down his entire system. Alright, though this
doesn't appear in the wild, that's worth a bumped severity.

  

  


Severity: normal → critical

    
    

    
  
So _is_ the <link> needed?  I have a fix that fixes the testcases in this bug by
making CSSLoader not do something dumb in a really hacky way, but that fix
assumes a bogus stylesheet link (as in those testcases), I think....  the
original site doesn't have such a beast, does it?

Note that I could maybe also fix the parser to bring back the old behavior...
maybe.  The old behavior had some issues, which I was sorta trying to fix.

  

  



    
    

    
  


  

  

  



    
    

    
  
Boris: every single tag in the reduced testcase is needed, even the arguments.

Please also note, that I used Mozilla 1.6 and Fierfox 0.8 as stable releases to
verify this hang.

Mozilla/5.0 (Windows; U; Windows NT 5.0; de-AT; rv:1.6) Gecko/20040113
Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.6) Gecko/20040206 Firefox/0.8

Maybe the bug is even older. I will test that later.

  

  



    
    

    
  
Boris: You can see the original website here: http://www.snej.de/tmp/test.php
(Will hang your system!)

The css is really bogus.

  

  



    
    

    
  
Er... I don't see a hang in builds older than 2004-05-06-08.  Are we talking
about the same bug?

And is the reduced testcase an actual reduction of the site, or just something
that shows a similar problem?  The testcase is basically showing something along
the lines of bug 197015 / bug 220542...

  

  



    
    

    
  
It can't hang my system, since I can kill the browser... ;)  Not to mention that
every single build I have on hand and am likely to test with is patched for this
bug at this point.  ;)

  

  



    
    

    
  
Comment on attachment 148243 [details] [diff] [review]
CSSLoader changes

We should just do this.  It's a terrible hack, but it should get us by till I
can fix the parser-blocking crap....

  

  



        Attachment #148243 -
        Flags: superreview?(peterv)

        Attachment #148243 -
        Flags: review?(peterv)

    
    

    
  
Though note that I've done very little testing of that patch, so if someone can
test it a bit, that would be nice....  I'm not too happy with checking it in
without said testing, but I won't have time to do it in the foreseeable future...

  

  



    
    

    
  

      
      
      
      

        Attached file
          
        original Website 
        — 
      

    


  
original Website the reduced testcases base on.

  

  



    
    

    
  
I just tested it with three browsers: Mozilla 1.6 , Mozilla 1.7 RC1 and Fx 0.8.
All of them hang. And of course I can also kill the preocess, but only if I'm
fast and aware of the fact, that the browser will eat all of my memory to the
point, that I can't use my system anymore.

But maybe I'm getting you terrible wrong at this. Do you mean, you don't see any
problems *with* the patch or do you say that everything was okay before
2004-05-06-08 and it hangs Mozilla only with current nightlies?

BTW: This is different than bug 197015 and bug 220542.
220542 will crash Mozilla and that's it. bug 197015 doesn't do anything at all
since it's "fixed" (somehow).
This bug does not crash Mozilla, but it eats so much memory, that your system
goes down.

  

  



    
    

    
  
(In reply to comment #15)
> But maybe I'm getting you terrible wrong at this. Do you mean, you don't see
> any problems *with* the patch or do you say that everything was okay before
> 2004-05-06-08 and it hangs Mozilla only with current nightlies?

The latter.  The 2004-05-05-08 nightly does not hang on any of the testcases in
this bug over here (Linux trunk gtk1 nightly and all that).

> BTW: This is different than bug 197015 and bug 220542.

Not on the code level it's not, at least when I can reproduce it -- it's a case
of a parser being resterted when it was never stopped.

> This bug does not crash Mozilla, but it eats so much memory, that your system
> goes down.

When I did see the "hang", the memory usage grew slowly enough that it would
take minutes to eat up the about 200MB that I was willing to let it have.

Sounds like there are at least two separate issues here, possibly...

  

  



    
    

    
  
> Sounds like there are at least two separate issues here, possibly...

Yes, you may be right with that. With all of my builds it ate more than 1.5 Gb
in only a few seconds.
But we are lucky: it's bug day. Many people with many different builds and
plattforms. I will ask for some help on IRC.

  

  



    
    

    
  
OK, I do see the fast memory leak with old enough builds. It's there in
2004-04-28-08 and not there in 2004-05-01-08.

So looks like fixing bug 240139 maybe fixed the issue, then fixing the
regressions from it "regressed" it (i.e. reintroduced it, by reintroducing the
old behavior).

  

  



    
    

    
  
Comment on attachment 148243 [details] [diff] [review]
CSSLoader changes

I don't think this patch is worth it if the problem is that old.

  

  



        Attachment #148243 -
        Flags: superreview?(peterv)

        Attachment #148243 -
        Flags: superreview-

        Attachment #148243 -
        Flags: review?(peterv)

        Attachment #148243 -
        Flags: review-
Depends on: 84582

    
    

    
  
> OK, I do see the fast memory leak with old enough builds. It's there
> in 2004-04-28-08 and not there in 2004-05-01-08.

Are you sure with that? I only contributed to that bug because of this post:
http://forums.mozillazine.org/viewtopic.php?p=520483#520483

He tried it with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a)
Gecko/20040509 Firefox/0.8.0+ and saw the same fast memmory fill

Wait: It's OS-dependend. I see a fast fill with Win2k and the same slow fill
like you on WinXP.

  

  



    
    

    
  
(In reply to comment #20)
> > OK, I do see the fast memory leak with old enough builds. It's there
> > in 2004-04-28-08 and not there in 2004-05-01-08.
> 
> Are you sure with that?

Absolutely.

> He tried it with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a)
> Gecko/20040509 Firefox/0.8.0+ and saw the same fast memmory fill

See comment 3.

> Wait: It's OS-dependend. I see a fast fill with Win2k and the same slow fill
> like you on WinXP.

Interesting.

  

  



    
    

    
  
Okay I have a wide ranging test (newest and oldest gecko browser on my computers):

Firefox 2004-05-11 0.8+ (rv1.8a)
using: reduced test case, 1gig of vm in about 30 second
pegging cpu usage to 100% in a few seconds

Camino 0.7 2003-11-21 (release build)
using: reduced test case, 1gig of vm in about 33 second

Mac OS X 10.3.3

  

  



    
    

    
  
One last item to note... same system same test case...

I also found an firebird build...
2004-01-25 gecko rev1.7a and found the same results... the reduced testcase eat
up to about 1.8G* of vm tasking the cpu around 75% then once it hit the 1.8 gig
mark cpu usage returns to about 5% and bounce between 5-15% until I kill of the
process. I see high pageing rates.

* I only have 4.41G of VM

  

  



    
    

    
  
I just tested this on my linux system, with the latest of firefox, 
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a) Gecko/20040511 Firefox/0.8.0+ ,
and it definitly crashes, i can't really test how fast the memory goes up,
becuase pretty instantly, it crashes, and my computer slows down to a lag and i
can barilly kill it.So in conclusion, this bug still lives, also on linux(very
badly) and needs and should be fixed for 1.7 as well everything else.


  

  



    
    

    
  
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a) Gecko/20040507 Firefox/0.8.0+
(daihard: XFT+GTK2; opt. for P4/SSE-2)

I get a fast memory leak when viewing the original pages or the testcase. In my
case Firefox ate nearly 95% of available physical RAM before I was able to shut
it down (I have 512MB of RAM and a 1137MB swap partition.)

  

  



    
  
Flags: blocking1.7? → blocking1.7-

    
    

    
  
Seems like one fix that could solve this problem is to clean up the parsers
handling of return codes from the DTD, sinks, observers, etc. Right now, I see
the parser contnously resuming off of its own events that it keeps firing again
and again and it gets to the sinks observers that by this time have no parser,
and returns an error code. As we unroll, that error code is lost, and we merely
think that the parser was interrupted, so we fire off an event to resume later,
only to hit the same case again, and again, and again...

Not something I'd want to deal with for 1.7...

  

  



    
    

    
  
Note that this bug contains a much safer patch that fixes bug 273953 (though
checking the other one in during an alpha cycle might not be a horrible idea).
Noting the dependancy.

  

  


Blocks: 273953

    
    

    
  
Sorry for the spam, I meant bug 220542

  

  


Blocks: 220542
No longer blocks: 273953

    
    

    
  
Comment on attachment 148243 [details] [diff] [review]
CSSLoader changes

For my future reference, the handling of mHaveBlockedParser when we glom on to
a heretofore-alternate load is wrong.  Since we call back into LoadSheet from
there, we could still fail and synchronously end up in SheetComplete.

  

  



    
    

    
  
I'm just going through my old bugs and this seems to be fixed. At least I can't
reproduce it anymore with current nightlies. Marking fixed.

  

  


Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED

    
    

    
  
No specific bug / patch referenced as the fix.

-> WORKSFORME


  

  



    
  
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
  
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → WORKSFORME
Status: RESOLVED → VERIFIED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: