Regression: "forever repeated" slow/unresponsive script warnings

VERIFIED FIXED in mozilla1.8.1beta2

Status

()

Core
JavaScript Engine
--
blocker
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Ria Klaassen (not reading all bugmail), Assigned: brendan)

Tracking

(4 keywords)

1.8 Branch
mozilla1.8.1beta2
fixed-seamonkey1.1a, hang, regression, verified1.8.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [verified-seamonkey1.1a])

Attachments

(3 attachments, 4 obsolete attachments)

Suddenly Firefox can't handle some scripts anymore.
For instance on this page: http://www.viva.nl/meepraten/forum/beauty/Viva.htm?NRMODE=Published&NRORIGINALURL=%2fmeepraten%2fforum%2fbeauty%2f&NRNODEGUID=%7b14921CA4-DFC2-4101-B098-3967A591196C%7d&NRCACHEHINT=Guest&AutoReturn=true and also the Menu Editor extension doesn't work anymore. You get a 100% CPU usage and after some time the warning.

Regression between 1.9a1_2006073112 and 1.9a1_2006073115:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2006-07-31+11%3A00&maxdate=2006-07-31+16%3A00
so the culprit could be Bug 345855 or Bug 346021 I guess.

I see the same on branch.
(Reporter)

Updated

12 years ago
Flags: blocking1.8.1?

Comment 1

12 years ago
Seeing this also on linux, when trying to use gmail.

OS -> All ?

Comment 2

12 years ago
Seeing this also on linux, when trying to use gmail.

OS -> All ?
(Reporter)

Comment 3

12 years ago
The value of dom.max_script_run_time makes no difference.
OS: Windows XP → All
Hardware: PC → All
(Reporter)

Updated

12 years ago
Severity: normal → blocker
(Reporter)

Comment 4

12 years ago
Hm, the first link is not a very good example. Probably the problematic script on that page is not always present.

Comment 5

12 years ago
Seeing this with Mac OS X trunk builds (Camino and Firefox) on Gmail
(Gmail never finish loading, requiring a force quit the browser to recover).

Comment 6

12 years ago
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060801 Minefield/3.0a1

Loading Gmail or Slashdot hangs.  When Slashdot hangs, the profiler in Activity Monitor seems to blame JavaScript iterator stuff.

Comment 7

12 years ago
Multiple slow script errors with 100% CPU on Gmail in Windows XP as well (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060801 Minefield/3.0a1 ID:2006080104 [cairo])
Does eventually finish loading but badly formatted (after 3 errors).

Comment 8

12 years ago
confirming, sucks :D
Also get several warnings on startup (no problem in safe mode, must be extensions or live bookmarks or stuff).

Comment 9

12 years ago
(In reply to comment #8)
> confirming, sucks :D
> Also get several warnings on startup (no problem in safe mode, must be
> extensions or live bookmarks or stuff).
Session restore IMHO. I got those startup warnings too. 
Trying to open Chatzilla also prompts the slow script - and there it hangs till closed.  Never gets logged in to mozdev.  

Of course Chatzilla did not work with yesterdays build either, don't know when it went broke, but that's another bug I'm sure.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060801 Minefield/3.0a1,Firefox ID:2006080104 [cairo]
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060801 SeaMonkey/1.1a] (nightly) (W98SE)

Other cases:
*Starting ChatZilla: I usually get 1-2 warning; now, CZ startup can't complete.
*<http://ns2966.ovh.net/flyspray/>: I'm not sure whether an account is needed to see the bug.
Summary: Regression: slow script warnings → Regression: "forever repeated" slow/unresponsive script warnings
Created attachment 231582 [details]
script that triggers the bug
(In reply to comment #12)
> Created an attachment (id=231582) [edit]
> script that triggers the bug
> 
Hm, only locally. :(

(In reply to comment #13)
> Hm, only locally. :(

Sure, Slashdot%20%20News%20for%20nerds,%20stuff%20that%20matters_files/prototype.js isn't there.

Comment 15

12 years ago
Created attachment 231585 [details]
Slashdot js 

Is this bug generally for big and complex xhtml+js sites?

Why did you add "forever repeated"? It's not forever repeated for me, I can stop it. slashdot is a good example, 2x stop script -> site loads

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060801 Minefield/3.0a1
Attachment #231582 - Attachment is obsolete: true
(In reply to comment #14)
> 
Yeah, it was still on my desktop. :)
The script shows the bug indeed but it is so long that I don't think it is useful to upload it.

Version: Trunk → 1.8 Branch
Flags: blocking1.9a1?
(In reply to comment #15)
> Why did you add "forever repeated"? It's not forever repeated for me, I can
> stop it.

I understand that it is not the case for every comment;
but for the 2 URI I mentionned, and my computer(...),
the sites won't load (at least not after quite a few "continue"),
and "stop" works for CZ, but not for flyspray (I "immediately" get another dialog);
thus I tried to describe the worst case: adding keywords, if you want.

Comment 18

12 years ago
I just had it hang every time. Session Restore would not restore things. Using a new session I would get a blank window that would go to 100% (nothing loaded) and I would get a dialog saying that the script is taking a long time to respond and selecting Kill it would go back to normal and then within a few seconds it would go to 100% again. 

I went into safe mode and after trying to load some urls (gmail) included it still hung. I tried to select some other options like "Debug" but that never took me anywhere. 

I've had to rollback to previous builds in order to be able to browse the web again. 
Confirming this is cased by the patch for bug 346021.  I backed it out locally and the problem has disappeared.

Updated

12 years ago
Assignee: general → brendan
(Reporter)

Updated

12 years ago
Keywords: hang

Comment 20

12 years ago
(In reply to comment #0)
> Suddenly Firefox can't handle some scripts anymore.

Also in MailNews at the SeaMonkey-Trunk. Switching with 'N' through 5 or 6 Postings, and the error comes up.

Comment 21

12 years ago
Here is a fantastic site that Firefox today cannot handle... http://www.mozilla.org/developer/


Oh dear!



Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060801 Minefield/1.5.1

Comment 22

12 years ago
Ok, add Local Install to the me-toos, but one things that has always irked me is that the Unresposive msg is useless if you are trying to track down what's unresponsive.  Several times I've said to myself, is it the page or the chrome interface, this bug of course seems to be the chrome interface, but an error or warning with more detailed information needs to be thrown into the error console like URL, line numbers function being executed, something, anything that will help pinpoint with having to hack the planet.

Should I bother submitting a RFE for this?

Comment 23

12 years ago
Same problem. screenshot attached.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060801 BonEcho/2.0b1

Comment 24

12 years ago
Created attachment 231600 [details]
sample from a hang loading the second attachment

sample taken using the "sample process" button in mac os x activity monitor.

Comment 25

12 years ago
(In reply to comment #22)
> [...]
> 
> Should I bother submitting a RFE for this?

I think bug 346648 is about what you want. 

Comment 26

12 years ago
Created attachment 231606 [details]
somewhat reduced testcase

From this file's dump() output, it looks like an iterator gets the property "setOptions" repeatedly.  I don't know why yet.

Comment 27

12 years ago
Created attachment 231616 [details]
more-reduced testcase
Attachment #231585 - Attachment is obsolete: true
Attachment #231606 - Attachment is obsolete: true
(Assignee)

Comment 28

12 years ago
Created attachment 231630 [details] [diff] [review]
fix

Major thanks to Jesse.

I blundered in collapsing cases in yesterday's patch, without making a matching change to the "walk up the prototype chain" enumeration logic.

You can see the bug with the extra context in this patch: if (!(flags & JSITER_ENUMERATE) || ...) we goto end_forinloop, so the successor basic block that tests (flags & JSITER_ENUMERATE) again is testing a condition that we know must be true.  In this case, I failed to store the current object along the prototype chain in vp[-1].  This is necessary given yesterday's patch to remove the code to get the current object from the parent slot of the native iterator.

The code is much clearer and simpler now: if we are going up the prototype chain, we must be "enumerating" (the backward-compatible for-in behavior that iterates over property ids, skipping properties with the DontEnum attribute; and that goes up the prototype chain, skipping "shadowed" ids; and that avoids iterating over ids that were deleted after the loop started).  When enumerating, we must call js_NewNativeIterator -- we do not want to call a custom __iterator__.  And (the fix), of course we must keep track of the current object along the prototype chain in vp[-1] (on the stack).

What makes all this hairy is keeping compatibility with "enumeration" while adding the new mode of "iteration", where custom __iterator__ settings help objects have more useful for-in loop behavior, which can iterate over arbitrary sequences of arbitrary values.

/be
Attachment #231630 - Flags: review?(mrbkap)
Attachment #231630 - Flags: approval1.8.1?
Comment on attachment 231630 [details] [diff] [review]
fix

>              * If the iterator is the iterable, do not expect to lookup fid

This comment is now out of date. r=mrbkap with that fixed.
Attachment #231630 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 30

12 years ago
Created attachment 231638 [details] [diff] [review]
with comment fixes
Attachment #231630 - Attachment is obsolete: true
Attachment #231638 - Flags: review?(mrbkap)
Attachment #231630 - Flags: approval1.8.1?

Updated

12 years ago
Attachment #231638 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 31

12 years ago
Comment on attachment 231638 [details] [diff] [review]
with comment fixes

This needs to go on the 1.8 branch right away.

/be
Attachment #231638 - Flags: approval1.8.1?
(Assignee)

Comment 32

12 years ago
Fixed on trunk.

/be
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Attachment #231638 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 33

12 years ago
Fixed on the 1.8 branch too.

/be
Keywords: fixed1.8.1
Target Milestone: --- → mozilla1.8.1beta2

Comment 34

12 years ago
Just updated to the latest - no longer getting this error. Looks good on first test. 

Comment 35

12 years ago
Both Gmail and Slashdot work in my trunk debug build now :)

Comment 36

12 years ago
Checking in regress-346801.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-346801.js,v  <--  regress-346801.js
initial revision: 1.1
Flags: in-testsuite+

Comment 37

12 years ago
*** Bug 346972 has been marked as a duplicate of this bug. ***

Comment 38

12 years ago
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060802 BonEcho/2.0b1.

Jesse's more-reduced testcase doesn't FAIL anymore.
Status: RESOLVED → VERIFIED
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Keywords: fixed1.8.1 → verified1.8.1
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060802 SeaMonkey/1.1a] (nightly) (W98SE)

V.Fixed on MOZILLA_1_8_BRANCH, for SeaMonkey too, per comment 11 cases.
Keywords: fixed-seamonkey1.1a
Whiteboard: [verified-seamonkey1.1a]

Comment 40

12 years ago
*** Bug 347368 has been marked as a duplicate of this bug. ***
*** Bug 347488 has been marked as a duplicate of this bug. ***
*** Bug 347204 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.