633879 - quora.com bloats out of control, part 4: wrapper proto keeps old windows alive

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Andreas Gal :gal]

+++ 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]

Comment 1

[Andreas Gal :gal]

I split off part 4. Fix for this in a sec.

Comment 2

[Andreas Gal :gal]

Patch in hand, asserts in chrome tests. Trying to reproduce.

Comment 3

[Andreas Gal :gal]

Attachment: patch

Comment 4

[Andreas Gal :gal]

Sent to try.

Comment 5

[Andreas Gal :gal]

isCrossCompartmentWrapper is broken, I removed it and replaced it with ->flags() & CROSS_COMPARTMENT. Sent to try again.

Comment 6

[Andreas Gal :gal]

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 7

[Andreas Gal :gal]

Attachment: patch

Comment 8

[Andreas Gal :gal]

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?

Comment 9

[Mike Shaver (:shaver emeritus)]

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.

Comment 10

[Andreas Gal :gal]

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

[Peter Van der Beken [:peterv]]

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

[Johnny Stenback (:jst)]

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.

Comment 13

[Johnny Stenback (:jst)]

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.

Comment 14

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/f8bec3eead64

Closing, please file new bugs.