Closed
Bug 310864
Opened 19 years ago
Closed 19 years ago
Greasemonkey 0.6.2 hangs on some pages since Firefox 1.4.1
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.8beta5
People
(Reporter: Mardak, Assigned: mrbkap)
References
Details
(Keywords: crash, regression, verified1.8)
Attachments
(4 files)
57.86 KB,
text/plain
|
Details | |
56.50 KB,
text/plain
|
Details | |
858 bytes,
patch
|
mrbkap
:
review+
mtschrep
:
approval1.8b5+
|
Details | Diff | Splinter Review |
79.69 KB,
text/plain
|
Details |
Loading gmail with a user script activated cause the browser to freeze. It's not specific to a particular user script because a blank one also causes the problem, so something with gmail and injecting scripts. (But it's not limited to just gmail.) Perhaps related to bug 310742? Aaron would probably be best to know what in greasemonkey might be causing this issue. Pretty sure it's caused by the fix for 308856 because backing out the changes causes this issue to go away. Last working build is Firefox 1.4 20050930.
Updated•19 years ago
|
Severity: normal → critical
Reporter | ||
Comment 1•19 years ago
|
||
Just as an update.. Applying the patch for bug 310742 (attachment 198385 [details] [diff] [review]) does not solve this issue. I had to back out the changes to jsapi.c/h and nsDOMClassInfo.cpp/h from bug 308856.
Reporter | ||
Comment 2•19 years ago
|
||
CCing brendan: "Taking from mrbkap so he can focus on WinXP drop shadow fixage."
Flags: blocking1.8b5?
Comment 3•19 years ago
|
||
Who says this has anything to do with that other bug? The only way to know is to get a stack, or sample of stacks if the process accumulates CPU time, and post them here. Can someone please do that? Another way (not for sure) is to binary search among past nightlies and even finer grained by date builds, and point to the regression window and its bonsai-reported checkin traffic. /be
Reporter | ||
Comment 4•19 years ago
|
||
Sorry if I wasn't clear with the bug description, but this started happening with Firefox 1.4.1 meaning 20051001 builds. This is definately related to bug 308856 because backing out those patches fixes this issue. I only guessed it was possibly related to bug 310742 because that also regressed from bug 308856, but I was hoping Aaron could clarify as he probably understands the greasemonkey code the best. Switching depends to 308856.. fixing 310742 didn't help.
Comment 5•19 years ago
|
||
Sorry, somehow misread your comment 0 about backing out the entire patch for bug 308856 curing this bug's symptom. Looks like I've got you on IRC, maybe we can gdb to get a stack.... /be
Reporter | ||
Comment 6•19 years ago
|
||
Stack trace from tiger's "report bug to apple"
Reporter | ||
Comment 7•19 years ago
|
||
Comment 8•19 years ago
|
||
Ok, thanks to some stacks from Edward, this is clearly a stack overflow. The JS engine should detect it and not crash. Not clear from the crash reports (in which the deep stack was cropped) what script is causing this stack overflow with a rogue toSource call. We really should find out, if possible. Look for a core file and run gdb on the app and that core file. /be
Assignee: general → brendan
Component: DOM → JavaScript Engine
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
Comment 9•19 years ago
|
||
(I mean its indirect callers, array_join_sub and js_obj_toSource -- because it recursively walks the graph being toSource'd in addition to those callers, which call themselves recursively to walk after marking on outermost call.) This is as safe a patch as you can get, given our stack limiting. There must be something GreaseMonkey-ish doing uneval() or obj.toSource() -- Aaron can you check? It's risky on deep graphs, but it shouldn't crash you as noted. It just might throw, so wrapping in try-catch helps, but why do it at all if you can avoid it? It's costly on large object graphs. /be
Attachment #198412 -
Flags: review?(mrbkap)
Attachment #198412 -
Flags: approval1.8b5?
Assignee | ||
Comment 10•19 years ago
|
||
Comment on attachment 198412 [details] [diff] [review] Have to check stack depth in MarkSharpObjects as well as its callers r=mrbkap
Attachment #198412 -
Flags: review?(mrbkap) → review+
Comment 11•19 years ago
|
||
Fixed on trunk. Plus'ing, this is trivially safe and helps an unchecked recursion path that can generate an overdeep stack. /be
Status: NEW → ASSIGNED
Flags: blocking1.8b5? → blocking1.8b5+
Updated•19 years ago
|
Attachment #198412 -
Flags: approval1.8b5? → approval1.8b5+
Comment 12•19 years ago
|
||
Fixed everywhere. Thanks for reporting this and helping on IRC. I changed the hang keyword to crash (if your OS makes a crash look like a hang, it has a "hang" bug ;-). /be
Reporter | ||
Comment 13•19 years ago
|
||
Just for comparison after applying the patch.. Loading mail.google.com (logged in) takes ~4 seconds with greasemonkey no scripts installed. After installing a blank user script (runs * but does nothing), mail.google.com loads in ~100 seconds. Which is definately better than never without the patch. :) [On a side note, it would take almost a minute before tiger killed firefox. I assumed it was hanging when I first ran into the issue, so I manually killed it before the autokill/crash.]
Reporter | ||
Comment 14•19 years ago
|
||
Aaron, here's a log with greasemonkey.logChrome set to true for loading gmail with a blank user script after applying the patch. The last function called before the loonnngggg sequence of "<<<Done enumerating" is.. GM_GreasemonkeyService.injectScripts([object Object], http://mail.google.com/mail/?view=page&name=js&ver=237a5136d7692b04, [object Window]) mrbkap pointed to the utils.js file line 391 which later uses args.join (But I'm not sure why it would cause this issue..)
Comment 15•19 years ago
|
||
This is not GreaseMonkey's fault. There's a separate bug here, in the split window code. We are marking the window.arguments object, whose '0' property is a window, whose 'window' property is the same window, but it keeps getting rewrapped by a new XPCWrappedNative, ad infinitum. I've filed bug 311022 on this. It should block 1.8 / Firefox 1.5. /be
No longer depends on: 311022
Comment 16•19 years ago
|
||
Reopening so this symptom report can catch dups. Should mark FIXED when bug 311022 is fixed. /be
Comment 17•19 years ago
|
||
mrbkap is all over this. He just said on IRC that it has to do with automatic implicit deep XPCNativeWrappers and window.window, window.window.window, etc. Aaron, can you jump on IRC today? /be
Assignee: brendan → mrbkap
Status: REOPENED → NEW
Reporter | ||
Comment 18•19 years ago
|
||
CCing Jeremy as he has worked quite a bit on greasemonkey as well. (And seems to be more available recently looking at the greasemonkey mailing list.) Could you help us out and stop by IRC?
Comment 19•19 years ago
|
||
I'll be able to get on IRC in a 2-3 hours. Unfortunately, my corp blocks IRC ports and I haven't gotten around to tunneling through it. Thanks for CC'ing me, Edward. Aaron's on very limited connectivity for the next week and a half.
Assignee | ||
Comment 20•19 years ago
|
||
Marking this fixed since bug 311022 is now fixed.
Status: NEW → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 21•19 years ago
|
||
Things are working like before now. :)
Status: RESOLVED → VERIFIED
Updated•19 years ago
|
Flags: testcase-
You need to log in
before you can comment on or make changes to this bug.
Description
•