Closed
Bug 633879
Opened 14 years ago
Closed 14 years ago
quora.com bloats out of control, part 4: wrapper proto keeps old windows alive
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: gal, Assigned: gal)
References
Details
(Keywords: memory-leak, regression, Whiteboard: [hardblocker], fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
+++ 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]
Assignee | ||
Comment 1•14 years ago
|
||
I split off part 4. Fix for this in a sec.
Assignee | ||
Updated•14 years ago
|
OS: Windows CE → All
Hardware: x86 → All
Updated•14 years ago
|
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
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → gal
Assignee | ||
Comment 2•14 years ago
|
||
Patch in hand, asserts in chrome tests. Trying to reproduce.
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Comment 4•14 years ago
|
||
Sent to try.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [hardblocker][has patch]
Assignee | ||
Comment 5•14 years ago
|
||
isCrossCompartmentWrapper is broken, I removed it and replaced it with ->flags() & CROSS_COMPARTMENT. Sent to try again.
Assignee | ||
Comment 6•14 years ago
|
||
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.
Assignee | ||
Comment 7•14 years ago
|
||
Attachment #512092 -
Attachment is obsolete: true
Assignee | ||
Comment 8•14 years ago
|
||
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.
Assignee | ||
Comment 10•14 years ago
|
||
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).
Comment 11•14 years ago
|
||
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[0]]-> 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 12•14 years ago
|
||
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+
Comment 13•14 years ago
|
||
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.
Updated•14 years ago
|
Whiteboard: [hardblocker][has patch] → [hardblocker]
Assignee | ||
Comment 14•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/f8bec3eead64
Closing, please file new bugs.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker] → [hardblocker], fixed-in-tracemonkey
Updated•13 years ago
|
Blocks: mlk-fx4-beta
You need to log in
before you can comment on or make changes to this bug.
Description
•