Browser should remain responsive during most infinite loops

NEW
Unassigned

Status

()

Core
General
--
major
18 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug, 4 keywords)

Trunk
arch, hang, helpwanted, perf
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

18 years ago
(Note: I present some presumptions here as facts.  Unless I totally blow 
something, please respond with non-flames.)


There have been many (exploitable) infinite loop bugs in mozilla, and their 
rate of appearance doesn't seem to be slowing down much.  Several involve 
layout: a reflow moves something down, which forces it to be moved back up, 
which forces it to be moved down again, etc.  Some involve javascript: perhaps 
there's a while(1) loop in the code.  Some involve history and page reloading.

Most of the time, the layout-related problems are fixed by reviewing the math 
that determines what happens when, leading to a fix specific to one type of 
infinite loop.  But HTML is complicated, and I doubt the layout engine will 
ever be safe from all types infinite loop attacks.

A better solution to this problem would be to add code in strategic places, 
such as reflow iterations, that force the browser to check if any user input 
(such as an attempt to close the browser) would make continuing the 
infinite/slow reflow moot.

In order to keep allow users to continue noticing reflow bugs (since they won't 
have to ctrl-alt-del the browser anymore), something should be added to the 
statusbar to make mozilla show when the browser is:

(1) idle
(2) waiting for the server to send (more) information
(3) waiting for user to respond to a modal dialog
(4) doing a normal reflow
(5) doing an incremental reflow
(6) executing javascript
(7) loading additional dlls or an external application

I propose that the spinning/moving bar on the left side of the statusbar be 
replaced or supplemented by a small box that changes color to indicate what the 
browser is doing.  Release versions might use fewer colors, perhaps light blue 
for 2 and 3, and dark blue for 4-7.  (This idea is based on a similar box in 
the dos game Civilization, which would switch between light grey and dark grey 
as the AI players went through various move phases and would get stuck on a 
specific color if a crash occurred.)

Users should quickly figure out that the browser is busy while the box/bar is a 
darker color because they won't be able to scroll, so they should still report 
infinite-reflow and similar bugs when they encounter them.


Some examples of bugs relating to infinite loops:
http://bugzilla.mozilla.org/buglist.cgi?bug_id=20283,24195,26618,30030,30830

Bug #20283  a href="javascript:history.go(0)"
Bug #24195  infinite reflow after adding content to scrollable frame
Bug #26618  infinite reflow in addressing ... more than one recipient
Bug #30030  infinite loop if pressing "back" leads to javascript redirect
Bug #30830  infinite loop in javascript code freezes mozilla

Some examples of slow (but not infinite) iteration problems:
http://bugzilla.mozilla.org/buglist.cgi?bug_id=17325,30091

Bug #17325  too many reflows on slashdot pages (with a fast connection)
Bug #30091  "select" form elements cause too many reflows
Dunno what the score is with confirmation of Enhancements, but this seems 
sensible, so confirming. It touches a lot of stuff, so leaving in 
Browser-General.

Gerv
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

18 years ago
infinite reflow is related to 31574, and that is a crasher as well.

Marking dupe, as solving that should fix this.


*** This bug has been marked as a duplicate of 31574 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE

Comment 3

18 years ago
ok, I was too quick on marking this a dupe.

moving to layout, think that might be the correct component.
Status: RESOLVED → REOPENED
Component: Browser-General → Layout
Resolution: DUPLICATE → ---
(Reporter)

Updated

17 years ago
Blocks: 25472

Comment 4

17 years ago
Bug #25472 was a joke.
No longer blocks: 25472
sounds like what is needed here is some sort of popup like IE's that says 
something to the effect of:

"A script on this page is taking a long time to complete execution. [ Continue ] 
[Stop running script ]"

(with better wording than that...)
(Reporter)

Comment 6

17 years ago
I think it would be better if mozilla could remain responsive (able to 
navigate, close, etc.) while the script is running.  Otherwise, malicious page 
authors are likely to find a way around the code that determines whether the 
script is hogging resources or preventing user interaction.

Comment 7

17 years ago
I guess I'll just throw in another suggestion: don't allow the user to interact
with the page while a script/alert box is running (edit text boxes, click on
links, activate mouseovers - essentially disabling the page UI itself, perhaps
with a transparent event sink like a glasspane over it), but *do* enable the
stop/back/forward buttons.  They can scroll through the page, but not modify
anything on it - something that could possibly screw up the script.  If the
action would invalidate the currently running script(hitting back/stop/forward,
entering a URL), pop up a warning saying so.

Comment 8

17 years ago
Adding myself to CC: list.

Comment 9

17 years ago
Adding crash keyword.
Keywords: crash
Moving to DOM Level 1 and reassigning.  I think this is a duplicate, though.
Assignee: cbegle → jst
Status: REOPENED → NEW
Component: Layout → DOM Level 1
QA Contact: asadotzler → gerardok
(Reporter)

Comment 11

17 years ago
This doesn't really belong to a single component afaict.  It's a general 
request for prevention against attacks (and other things) that cause the 
browser to freeze.  It would need to be done in layout, javascript, and plenty 
of other places.  If you guys decide to fix it, this bug would probably become 
a meta-bug, with dependencies in various components.

cc mstoltz@netscape.com, the security person.

consider changing summary from "Infinite reflows, loops should not crash 
browser" to "Browser should remain responsive during most infinite loops"

Comment 12

17 years ago
> A better solution to this problem would be to add code in strategic places, 
> such as reflow iterations, that force the browser to check if any user input 
> (such as an attempt to close the browser) would make continuing the 
> infinite/slow reflow moot.

I completely agree. You should always insert calls to the event loop after some
ms. It is not even hard to implement, if you e.g. call it once during a reflow
and in any central place in the compiled JS code. It should be done soon,
though, because this might have some side-effects, so needs testing.


Changing SUMMARY to davidr's suggested one.

Assuming davidr's claims are correct (especially that |while(1)| in a web script
or infinite reflows cause a freeze), I don't think, this is an enhancement
request, but a design bug, changing SEVERITY to major.

I suggest to file separate bugs for layout, JS and so on.
Severity: enhancement → major
Summary: Infinite reflows, loops should not crash browser → Browser should remain responsive during most infinite loops
This bug has been marked "future" because the original netscape engineer
workingon this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work in
some way -- please attach your concern to the bug for reconsideration.
Status: NEW → ASSIGNED
Target Milestone: --- → Future

Updated

17 years ago
Keywords: helpwanted

Comment 14

17 years ago
This bug blocks some infinite reflow loops in chatzilla. Bug 45576
Blocks: 45576

Comment 15

17 years ago
Bug 45576 - Hang on joining #mozillazine
I'm concerned. I've marked helpwanted. If you don't like this bug I'll assign 
it to nobody.  Added keywords, nothing in this world gets done w/o nsbeta3.

Bug reassigned to default victim. Again, if you don't like this bug please send 
it somewhere. nobody is asking for it.

This is a catchall, but Ben's right, we should at the very least be able to do 
time counts on threads.

init does stuff like this, if it needs to respawn the console tty too many 
times it throttles it [odd case: no user is avail to prompt because interface 
is being throttled, but ...].
Assignee: jst → rpotts
Status: ASSIGNED → NEW
Component: DOM Level 1 → Threading
Keywords: arch, nsbeta3
QA Contact: gerardok → rpotts
Target Milestone: Future → ---

Comment 16

17 years ago
> Ben's right, we should at the very least be able to do time counts on threads

What I meant was no counter, but to make sure, the event loop is called by just
inserting calls at the right place. With only that done (rightTM), the infinite
loops would still occur, but the app wouldn't freeze, which is already a
progress. More importantly, even in normal operation, the app were more
responsive. Maybe, my suggestion was orthogonal to this bug, dunno.

Updated

17 years ago
Whiteboard: [nsbeta3+]

Updated

17 years ago
Target Milestone: --- → M18
(Reporter)

Comment 17

17 years ago
I don't know how appropriate it is to say that this bug "blocks" various freeze 
bugs.  Having this feature implemented well would make mozilla remain 
responsive during freezes, so, for example, you could close a slow-rendering 
webpage without killing mozilla.  Most of the individual frezze bugs should 
still be fixed, however, because they might be triggered by non-malicious 
webpages that we actually want to display completely.  Instead of being 
critical/crash bugs, they would become major or normal bugs (with the perf 
keyword?), but they'd still be there.

Comment 18

17 years ago
Seems like no (or too few) event loop calls are done during the frame
construction.

Comment 19

17 years ago
minusing per PDT rules.
Whiteboard: [nsbeta3+] → [nsbeta3-]

Updated

17 years ago
Keywords: mozilla1.0

Comment 20

17 years ago
I don't think this is a crash bug.

Comment 21

17 years ago
*** Bug 51945 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Depends on: 62778
(Reporter)

Updated

17 years ago
Depends on: 13350

Updated

17 years ago
Target Milestone: M18 → ---

Updated

17 years ago
Target Milestone: --- → Future

Updated

17 years ago
Keywords: perf
(Reporter)

Updated

16 years ago
Depends on: 73272

Updated

16 years ago
Keywords: nsbeta3
Whiteboard: [nsbeta3-]
(Reporter)

Updated

16 years ago
Depends on: 31041

Comment 22

16 years ago
*** Bug 98087 has been marked as a duplicate of this bug. ***

Comment 23

16 years ago
I have one comment: doing recursive calls to event handling can cause hard to
find/reproduce bugs.

Has anyone considered the possibility to run XUL in one thread and the HTML in
another one? They should be largely separate worlds because of security if
nothing else. (No sharing of JS contexts or anything.)

Updated

16 years ago
Blocks: 99053
(Reporter)

Updated

16 years ago
No longer blocks: 17325, 20283, 24195, 26618, 30030, 30091, 30830, 45576

Comment 24

16 years ago
I want to add in general that keeping the browser responsive at all times should
become a major priority, not just to deal with infinite loops, but to deal with
all sorts of delays.  For example, I have broadband and I typically set my
Google preferences set to return 100 items per page --- this greatly reduces the
time spent hitting the "next" button when I am looking for content.  However,
using Mozilla it can take several seconds before the browser decides to allow me
to click on a link in the search.  What's even more annoying is that when I hit
the "back" button to return to the search page, again the page is unresponsive
for a few seconds.  This on a Pentium II 400 laptop with 192 megs of RAM. 
Suffice it to say that IE performs much better in this respect, and this is one
of the main subjective performance issues that differentiates IE from Mozilla.

This occurs not only with Google searches but with many pages: links are not
active, it is impossible to even scroll a page, etc.  There are far too many
situations where potentially long processes that should be handled in the
background are completely tying up the main user interface thread.

Fixing this problem might require a great deal of work, I don't know, but I
think it ought to be a priority.
Mitsu,
   Increasing performance and responsiveness is our top priority, as a matter of
fact. Thanks for your comments. However, this discussion is not relevant to this
particular bug - we try to keep discussion in bugs focused on the specific issue
at hand.

Comment 26

16 years ago
Sorry --- I had originally thought to enter a new bug, but I figured it would be
better to find a related bug and enter the comment there.  It seemed to me
(perhaps I am wrong) that in general, a single general approach would solve both
the problem of keeping the browser responsive during infinite loops and during
any extended operations.  I.e., the general principle being that nothing should
hold up the user interface thread for more than a very brief interval, and all
operations should be thread-safe.

If there is more appropriate bug or RFE which is already open where I should
place these comments, I'd be happy to do so.
See also bug 128666 about showing the display during tight loops.
*** Bug 128666 has been marked as a duplicate of this bug. ***

Comment 29

15 years ago
This bug is very visible on large doxygen output with the tree browser. I've
created an example at <http://apt.slamb.org:8080/~slamb/std-doxygen/> (the
standard C++ library as included with g++ 3.0.4). It takes 270 seconds to load
on my 1GHz Athlon with 768MB of RAM. During that time, Mozilla is completely
unresponsive - it doesn't even handle expose events.
(Reporter)

Comment 30

15 years ago
Scott: that's a general problem with loading content from a hard drive, from a
fast server, or from the cache.  See bug 129640, "Incremental rendering is
busted on fast connections".

Comment 31

15 years ago
The problem also occurs when loading over a modem. The problem is definitely
related to the Javascript that creates the tree frame. You haven't loaded the
page - give it a try and you'll see what I mean. If you disable Javascript, it
will load nearly instantly (without the tree). I think this is a good match for
(6) in this bug's description.
(Reporter)

Comment 32

15 years ago
Sorry Scott, you're right.
*** Bug 150593 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Keywords: mozilla1.0 → mozilla1.2
Moving all threading bugs to XPCOM. See bug 160356.
Component: Threading → XPCOM

Comment 35

15 years ago
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.
Severity: major → critical

Updated

14 years ago
Keywords: mozilla1.2
(Reporter)

Updated

14 years ago
Keywords: crash → hang
QA Contact: rpotts → xpcom
(Reporter)

Updated

10 years ago
Depends on: 40848, 308158
Assignee: rpotts → nobody

Updated

10 years ago
Blocks: 384115

Updated

10 years ago
No longer blocks: 384115

Updated

10 years ago
Blocks: 384323
Severity: critical → major
Component: XPCOM → General
Priority: P3 → --
QA Contact: xpcom → general
Target Milestone: Future → ---

Updated

6 years ago
No longer blocks: 99053
You need to log in before you can comment on or make changes to this bug.