57.86 KB, text/plain
56.50 KB, text/plain
858 bytes, patch
|Details | Diff | Splinter Review|
79.69 KB, text/plain
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.
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.
CCing brendan: "Taking from mrbkap so he can focus on WinXP drop shadow fixage."
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
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.
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
Stack trace from tiger's "report bug to apple"
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
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
(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
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+
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+
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
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Keywords: hang → crash, fixed1.8
Resolution: --- → FIXED
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.]
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..)
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
Reopening so this symptom report can catch dups. Should mark FIXED when bug 311022 is fixed. /be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
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?
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.
Marking this fixed since bug 311022 is now fixed.
Status: NEW → RESOLVED
Last Resolved: 14 years ago → 14 years ago
Resolution: --- → FIXED
Things are working like before now. :)
Status: RESOLVED → VERIFIED
verifying per Edward's comment
Keywords: fixed1.8 → verified1.8
You need to log in before you can comment on or make changes to this bug.