Closed Bug 633879 Opened 9 years ago Closed 9 years ago
.com bloats out of control, part 4: wrapper proto keeps old windows alive
+++ This bug was initially created as a clone of Bug #630072 +++ Numbers for linux32 bit running w/ Flash: Memory according to Ubuntu about:home - 27M quora.com - 52M after 200 quora DOMWindows - 430-460M The initial load of Quora isn't bad. It's actually quite heavy in all browsers. But after clicking around for 200 DOMWindows in one tab in a debug build, our heap has grown out of control. Reading my debug logs, I don't see DocShell bloat, but I do see DOMWindow bloat. I'll attach my shutdown log in a debug build. Notice the build shuts down cleanly (no leaks), but DOMWindows with very old serial numbers are still alive at shutdown. The last 10 DOMWindows destroyed are a grab bag: --DOMWINDOW == 10 (0xad2cacc) [serial = 44] [outer = (nil)] [url = http://www.quora.com/Is-Mexico-a-part-of-North-America-or-Central-America] --DOMWINDOW == 9 (0xbac54c4) [serial = 89] [outer = (nil)] [url = http://www.quora.com/Urban-Dictionary] --DOMWINDOW == 8 (0x9a671834) [serial = 62] [outer = (nil)] [url = http://www.quora.com/Why-might-someone-think-that-being-called-a-gentleman-would-be-bad] --DOMWINDOW == 7 (0x987fdc6c) [serial = 123] [outer = (nil)] [url = http://www.quora.com/Android-Market/Are-there-any-tools-that-help-Android-developers-monitor-sales] --DOMWINDOW == 6 (0xc139a54) [serial = 100] [outer = (nil)] [url = http://www.quora.com/If-I-owned-a-domain-name-can-I-use-Posterous-iPhone-app-to-blog-there] --DOMWINDOW == 5 (0xb3d40c4) [serial = 78] [outer = (nil)] [url = http://www.quora.com/Why-is-calamansi-not-in-the-Merriam-Webster-dictionary] --DOMWINDOW == 4 (0x94c7485c) [serial = 133] [outer = (nil)] [url = http://www.quora.com/What-are-the-best-social-media-monitoring-product-websites] WARNING: NS_ENSURE_TRUE(!(mAsyncExecutionThread)) failed: file /home/sayrer/dev/mozilla-central/storage/src/mozStorageConnection.cpp, line 674 --DOMWINDOW == 3 (0x9a31ecc) [serial = 7] [outer = (nil)] [url = http://www.quora.com/Business-Development] --DOMWINDOW == 2 (0x9ecf0a74) [serial = 112] [outer = (nil)] [url = http://www.quora.com/Where-do-I-find-investors-for-an-Internet-startup-idea] --DOMWINDOW == 1 (0xa4af277c) [serial = 200] [outer = (nil)] [url = http://www.quora.com/Business-Development] --DOMWINDOW == 0 (0xe3781e4) [serial = 192] [outer = (nil)] [url = http://www.quora.com/Business-Development]
I split off part 4. Fix for this in a sec.
Summary: quora.com bloats out of control, part 4: wrapper proto keeps old window salive → quora.com bloats out of control, part 4: wrapper proto keeps old windows alive
Patch in hand, asserts in chrome tests. Trying to reproduce.
Sent to try.
isCrossCompartmentWrapper is broken, I removed it and replaced it with ->flags() & CROSS_COMPARTMENT. Sent to try again.
Here is the assert jst got: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1297664639.1297667145.2171.gz Command line to reproduce: python runtests.py --chrome --test-path=js/src/xpconnect/tests/chrome --debugger=gdb The wrapper is: (gdb) x/a $2->getProxyHandler() 0x7ffff7bb9e40 <_ZN25JSCrossCompartmentWrapper9singletonE>: 0x7ffff7b006d0 <_ZTV25JSCrossCompartmentWrapper+16> (gdb) p $2->getProto() $3 = (JSObject *) 0x7fffd2128478 (gdb) call js_DumpObject($3) object 0x7fffd2128478 class 0x7ffff7b6fa00 Proxy flags: delegate not native proto <XPC_WN_ModsAllowed_NoCall_Proto_JSClass object at 0x7fffd212b9f8> parent <Window object at 0x7fffd2123798> slots: The prototype of the wrapper is a window object. This is a weird sandbox wrapper. This is really ugly. Wrappers should only have wrappers as proto. I will stop the parent-adjustment-along-the-proto-chain once we hit a non-wrapper object.
Comment on attachment 512123 [details] [diff] [review] patch peterv, can you please review the change in the bug? (abort if we hit a non-wrapper along the proto chain) and then land the patch for me? And then lets see if we still leak with your editor patch. I can't reproduce any InstallTrigger-related leaks with this applied (plus the patch I already landed on TM). If you still see us keep InstallTrigger alive I need some STR. You said its the getter somehow?
Attachment #512123 - Flags: review?(peterv)
Can I help schedule a call between all the relevant folks on these 4 bugs, so that we work through remaining issues more synchronously? Time isn't on our side, and hours-latency of bugmail responses isn't on the side of time.
Peterv and I have a round-the-clock thing going on here. I handed off at 2am. I am back to collect results (see other bug, leak is fixed).
I've been trying to track down a remaining window leak, but still working on figuring it out. --[XPCWrappedNativeScope::mGlobalJSObject]-> 0x7fffccdac2d0 [JS Object (Window) (global=7fffccdac2d0)] --[InstallTrigger getter]-> 0x7fffccdd0c38 [JS Object (Proxy) (global=7fffccdac2d0)] --[wrappedObject]-> 0x7fffd1a7de70 [JS Object (Function) (global=7fffdeb35f30)] --[upvars]-> 0x7fffdebba0d0 [JS Object (Proxy) (global=7fffe0734510)] --[proto]-> 0x7fffd4762e38 [JS Object (Proxy) (global=7fffe0734510)] --[proto]-> 0x7fffd4762dd0 [JS Object (Proxy) (global=7fffe0734510)] --[wrappedObject]-> 0x7fffd47701b0 [JS Object (Object) (global=7fffd4770120)] --[parent]-> 0x7fffd4770120 [JS Object (Window) (global=7fffd4770120)] --[xpc_GetJSPrivate(obj)]-> 0x7fffd80ba3c0 [XPCWrappedNative (Window)] --[mIdentity]-> 0x7fffd4427878 [nsGlobalWindow]
Comment on attachment 512123 [details] [diff] [review] patch r=jst, and gal says mrbkap reviewed all of this except the last bit about stopping when finding a non-wrapper on the proto chain.
Attachment #512123 - Flags: review?(peterv) → review+
Andreas' patch landed: http://hg.mozilla.org/mozilla-central/rev/9e7a0e7f1f61 Leaving bug open for now to track any remaining issues, or feel free to file new bugs if that's more appropriate here.
http://hg.mozilla.org/tracemonkey/rev/f8bec3eead64 Closing, please file new bugs.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker] → [hardblocker], fixed-in-tracemonkey
You need to log in before you can comment on or make changes to this bug.