Closed Bug 633672 Opened 9 years ago Closed 9 years ago

quora.com bloats out of control (part 2)

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 633382
Tracking Status
blocking2.0 --- final+

People

(Reporter: gal, Assigned: peterv)

References

Details

(Keywords: memory-leak, regression, Whiteboard: [hardblocker])

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #630072 +++

This bug will investigate the remaining quora.com leak.
Blocks: 630072
No longer depends on: 630072
Andreas, I'm still seeing the InstallTrigger getter hold an old window alive
with a slightly different path:

        --[InstallTrigger getter]-> 0x7fffcd03bcc0 [JS Object (Proxy)
(global=7fffcd066a20)]
        --[wrappedObject]-> 0x7fffd21ea4d0 [JS Object (Function)
(global=7fffdeb35f30)]
        --[upvars[0]]-> 0x7fffdebba0d0 [JS Object (Proxy)
(global=7fffdaa6c0d8)]
        --[proto]-> 0x7fffdaa93340 [JS Object (Proxy) (global=7fffdaa6c0d8)]
        --[wrappedObject]-> 0x7fffd475d108 [JS Object
(XPC_WN_ModsAllowed_NoCall_Proto_JSClass - Window) (global=7fffd475b120)]
        --[parent]-> 0x7fffd475b120 [JS Object (Window) (global=7fffd475b120)]
        --[xpc_GetJSPrivate(obj)]-> 0x7fffe1cb3ac0 [XPCWrappedNative (Window)]
        --[mIdentity]-> 0x7fffd46a5078 [nsGlobalWindow]
This is still InstallTrigger, so its still a regression and a hardblocker. There is a 3rd part to this that is an editor leak which is probably not a regression, so that would be a softblocker. We have to fix this first to tell whats up.
Whiteboard: [hardblocker]
Is there a bug on the editor thing?  Peter and I were talking about it today and have a good idea for what will fix it...
Blocks: 633738
Blocks: 633879
Assignee: nobody → gal
Again:

0x7fffcfc0e9d8 [JS Object (Window) (global=7fffcfc0e9d8)]
--[InstallTrigger getter]-> 0x7fffcfc2b440 [JS Object (Proxy) (global=7fffcfc0e9d8)]
--[wrappedObject]-> 0x7fffe2af1d10 [JS Object (Function) (global=7fffdeb35f30)]
--[upvars[0]]-> 0x7fffdebba0d0 [JS Object (Proxy) (global=7fffe0734510)]
--[proto]-> 0x7fffdebf5d00 [JS Object (Proxy) (global=7fffe0734510)]
--[proto]-> 0x7fffdebf5c98 [JS Object (Proxy) (global=7fffe0734510)]
--[wrappedObject]-> 0x7fffe2a061b0 [JS Object (Object) (global=7fffe2a06120)]
--[parent]-> 0x7fffe2a06120 [JS Object (Window) (global=7fffe2a06120)]
--[xpc_GetJSPrivate(obj)]-> 0x7fffe4034040 [XPCWrappedNative (Window)]
--[mIdentity]-> 0x7fffd4ccd478 [nsGlobalWindow]

Starting at the object stored in upvars[0]:

(gdb) p (JSObject*)0x7fffdebba0d0
$10 = (JSObject *) 0x7fffdebba0d0
(gdb) call js_DumpObject($)
object 0x7fffdebba0d0
class 0x7ffff7b6e8c0 Proxy
flags: delegate
not native
proto <Proxy object at 0x7fffdebf5d00>
parent <ChromeWindow object at 0x7fffe0734510>
slots:

(gdb) x/a $->getProxyHandler()
0x7ffff7b945e0 <_ZN3xpc11XrayWrapperI25JSCrossCompartmentWrapperE9singletonE>:	0x7ffff7a47290 <_ZTVN3xpc11XrayWrapperI25JSCrossCompartmentWrapperEE+16>
(gdb) p $->getProxyPrivate().toObjectOrNull()
$11 = (JSObject *) 0x7fffdebb5068
(gdb) call js_DumpObject($)
object 0x7fffdebb5068
class 0x7ffff7b6ea00 Proxy
flags: delegate
not native
proto <XPC_WN_ModsAllowed_NoCall_Proto_JSClass object at 0x7fffcfc2c370>
parent <Window object at 0x7fffcfc0e9d8>
slots:

(gdb) x/a $->getProxyHandler()
0x7ffff7b8ed30 <_ZN18nsOuterWindowProxy9singletonE>:	0x7ffff79c2f90 <_ZTV18nsOuterWindowProxy+16>

So looks like 0x7fffdebba0d0 is a wrapper around an outer window. The proto of the outer window is 0x7fffcfc2c370

If I go up the proto chain for the proxy:

(gdb) p $10->proto
$12 = (JSObject *) 0x7fffdebf5d00
(gdb) x/a $->getProxyHandler()
0x7ffff7b94640 <_ZN3xpc15NoWaiverWrapper9singletonE>:	0x7ffff7a46a30 <_ZTVN3xpc15NoWaiverWrapperE+16>
(gdb) p $->getProxyPrivate().toObjectOrNull()
$13 = (JSObject *) 0x7fffdebb3108

The proto of the wrapper is a wrapper around a different object than the proto of the outer window.

(gdb) call js_DumpObject($)
object 0x7fffdebb3108
class 0x7ffff7b52f60 XPC_WN_ModsAllowed_NoCall_Proto_JSClass
flags: delegate own_shape inDictionaryMode hasPropertyTable
properties:
    "__defineSetter__": slot 11
    "__defineGetter__": slot 10
    "__lookupSetter__": slot 9
    "__lookupGetter__": slot 8
    enumerate "dump": slot 7
    enumerate "scrollByLines": slot 6
    enumerate "getSelection": slot 5
    enumerate shared "top": slot -1
    enumerate shared "parent": slot -1
    enumerate shared "name": slot -1
    enumerate "addEventListener": slot 4
    enumerate "removeEventListener": slot 3
    enumerate "dispatchEvent": slot 2
    enumerate "getComputedStyle": slot 1
    enumerate shared "localStorage": slot -1
    enumerate shared "globalStorage": slot -1
    enumerate shared "sessionStorage": slot -1
proto <Object at 0x7fffe2a061b0>
parent <Window object at 0x7fffe2a06120>
private 0x7fffe1c99b40
slots:
   0 (reserved) = undefined
   1 = <function getComputedStyle at 0x7fffdebb2400 (JSFunction at 0x7fffdebb2400)>
   2 = <function dispatchEvent at 0x7fffdebb2480 (JSFunction at 0x7fffdebb2480)>
   3 = <function removeEventListener at 0x7fffdebb2500 (JSFunction at 0x7fffdebb2500)>
   4 = <function addEventListener at 0x7fffdebb2580 (JSFunction at 0x7fffdebb2580)>
   5 = <function getSelection at 0x7fffdebb2780 (JSFunction at 0x7fffdebb2780)>
   6 = <function scrollByLines at 0x7fffdebb2800 (JSFunction at 0x7fffdebb2800)>
   7 = <function dump at 0x7fffdebb2700 (JSFunction at 0x7fffdebb2700)>
   8 = <function __lookupGetter__ at 0x7fffdebb2880 (JSFunction at 0x7fffdebb2880)>
   9 = <function __lookupSetter__ at 0x7fffdebb2900 (JSFunction at 0x7fffdebb2900)>
  10 = <function __defineGetter__ at 0x7fffdebb2980 (JSFunction at 0x7fffdebb2980)>
  11 = <function __defineSetter__ at 0x7fffdebb2a00 (JSFunction at 0x7fffdebb2a00)>
I found the issue. I've been hunting for the place where we changed the proto of the window objects, but after a lot of printf debugging and coming up with nothing it hit me that we're actually not doing that, we're transplanting. Turns out that when we transplant same-compartment we don't do anything about cross-compartment wrappers in other compartments. It makes sense, since we really replace the guts of the existing object with a new one and cross-compartment wrappers can keep pointing to the existing object. However, transplanting means that the proto of the object changes (it's part of the guts) so we change the proto to one that's parented to a different object, but the proto of the cross-compartment wrappers aren't updated. Once I added code to go through all cross-comparment wrappers and update their proto to a cross-compartment wrapper of the new proto the leak went away. I'll attach my patch.
Gal, does this make sense?
Attached patch v1Splinter Review
Assignee: gal → peterv
Status: NEW → ASSIGNED
Attachment #512496 - Flags: feedback?(gal)
Excellent find Peter! I think we probably need the same treatment in js_TransplantObjectWithWrapper() (landed just a few days back), no?
peterv, I think the proper fix for this is bug 633382. If you still see this edge, please re-open this bug.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 633382
Yeah, seems fixed now.
Attachment #512496 - Flags: feedback?(gal)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.