691537 - initializing add-on installation prevents releasing some memory, even if installation dismissed, [pending install]

Status: RESOLVED WORKSFORME

(Toolkit :: Add-ons Manager, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111001 Firefox/10.0a1
Also with 7.0.1

Initializing an add-on installation prevents the release of some memory affiliated with the page from which the add-on is being downloaded, even if the installation has been dismissed.

Steps to reproduce:
1. Open https://addons.mozilla.org
2. Search for Adblock Plus (likely compatible with trunk builds).
3. Click the install button next to it.
4. Either click the install button or dismiss it.
5. Open about:memory.
6. Click the 'More verbose' link.
7. Run the 'Minimize memory usage' three times.

Actual result:
There is still a compartment for addons.mozilla.org.

Expected result:
No compartment for addons.mozilla.org

Additional information:
This also happens on stable with clicking xpi links on http://releases.mozilla.org/pub/mozilla.org/firefox/releases/7.0.1/win32/xpi/

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So is this about the Add-ons Manager or Spidermonkey?

Comment 2

[Dave Townsend [:mossop]]

Difficult to say right now. What is the state of the art of tools for looking at what is left in a compartment?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Dave Townsend (:Mossop) from comment #2)
> Difficult to say right now. What is the state of the art of tools for
> looking at what is left in a compartment?

cc'ing Blake, who probably has an answer for us available.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

CC'ing the dudes who eat leaks for breakfast.

Comment 5

[Andrew McCreight [:mccr8]]

I can try doing some cycle collector / JS heap dumping stuff to figure out what is keeping the compartment alive.

Comment 6

[Andrew McCreight [:mccr8]]

For me, the reproduction steps seem to be:
  5. Open about:memory in a new tab, then close the addons.mozilla.org tab.
  6. Click the 'More verbose' link.
  7. Run the 'Minimize memory usage' three times.

If I open about:memory in the original tab containing addons.mozilla.org, then the compartment goes away as expected.

Comment 7

[Andrew McCreight [:mccr8]]

I tried applying some experimental tools to this.  Basically, I produce a list of objects in other compartments that contain pointers to the addons compartment.  I also have a list of objects from the addons compartment that are JS roots.  All of the latter are gray roots, so I'm not sure they matter or not.

Anyways, all of the pointers from other compartments seem to follow this path:

via nsXPCWrappedJS[nsISupports,0x124b96380:0x1270eca40].mJSObj :
0x12538cf30 [Object <no private>]
    0x117a3d000 --[installs]-> 0x1253475b0 [Array 1]
    0x117a3d000 --[element[0]]-> 0x12538cab8 [Object <no private>]
    0x117a3d000 --[shape]-> 0x1210cf340 [shape 85393]
    0x117a3d000 --[linkedInstalls getter]-> 0x12534f088 [Function 118332a80]
    0x117a3d000 --[upvars[0]]-> 0x12538ca50 [Object <no private>]
    0x117a3d000 --[listeners]-> 0x117fb9450 [Array 1]
    0x117a3d000 --[element[0]]-> 0x12538cb88 [Object <no private>]
    0x117a3d000 --[onInstallFailed]-> 0x12534f558 [Function 117feda80]
    0x117a3d000 --[parent]-> 0x12531aec8 [Call 0]
    0x117a3d000 --[parent]-> 0x12531a848 [Call 0]
    0x117a3d000 --[parent]-> 0x120cad2b8 [Call 0]
    0x117a3d000 --[aWindow]-> 0x11ac143d0 [Proxy <no private>]

via Preserved wrapper :
0x125309b88 [XULElement 11a77a510]
    0x117a3d000 --[notification]-> 0x12536aec8 [Object <no private>]
    0x117a3d000 --[options]-> 0x12536a918 [Object <no private>]
    0x117a3d000 --[installs]-> 0x1253475b0 [Array 1]
    0x117a3d000 --[element[0]]-> 0x12538cab8 [Object <no private>]
    0x117a3d000 --[shape]-> 0x1210cf340 [shape 85393]
    0x117a3d000 --[linkedInstalls getter]-> 0x12534f088 [Function 118332a80]
    0x117a3d000 --[upvars[0]]-> 0x12538ca50 [Object <no private>]
    0x117a3d000 --[listeners]-> 0x117fb9450 [Array 1]
    0x117a3d000 --[element[0]]-> 0x12538cb88 [Object <no private>]
    0x117a3d000 --[onInstallFailed]-> 0x12534f558 [Function 117feda80]
    0x117a3d000 --[parent]-> 0x12531aec8 [Call 0]
    0x117a3d000 --[parent]-> 0x12531a848 [Call 0]
    0x117a3d000 --[parent]-> 0x120cad2b8 [Call 0]
    0x117a3d000 --[aWindow]-> 0x11ac143d0 [Proxy <no private>]

The "onInstallFailed" edge looks suspicious.  It comes from this object:

0x12538cb88 0x117a3d000 Object <no private>
> 0x117fd37c0 type
> 0x117f30830 parent
> 0x1210cad00 shape
> 0x12534f450 onDownloadCancelled
> 0x12534f500 onDownloadFailed
> 0x12534f558 onInstallFailed
> 0x12534f5b0 onInstallEnded

It looks like that is this function: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/addonManager.js#168

Comment 8

[Andrew McCreight [:mccr8]]

That was just from looking at the JS GC heap.

Looking at the cycle collector graph produces this:

0x118a30ca0 [nsGenericElement (XUL) popupnotification]
    --[Preserved wrapper]-> 0x125309b88 [JS Object (XULElement) (global=1188338f8)]

The XULelement that is eventually holding onto the aWindow proxy is being held onto by a XUL element popupnotification, which also looks suspicious.  This popupnotification is being kept alive because of one unknown reference.  The popupnotification goes away at the next cycle collection, but the known references at that point are a subset of the one where it doesn't go away, so the unknown reference went away without being seen by the cycle collector.

Note that for the cycle collector graph, there are a bunch of things holding onto the XUL element 0x125309b88, so it is hard to say if figuring out this one thing would be enough.

Comment 9

[Andrew McCreight [:mccr8]]

Attachment: complete set of objects keeping 0x125309b88 alive

Comment 10

[Andrew McCreight [:mccr8]]

I spent some more time digging around in the heap here.  What is happening is that the object containing a bunch of callbacks (onDownloadCancelled, onDownloadFailed, etc.) is being installed as a listener: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/addonManager.js#156

All of these callbacks capture their enclosing callers, which leads back eventually to the scope for AMC_installAddonsFromWebpage, which includes the argument aWindow, which is a cross-compartment reference keeping the addons compartment alive.

I don't really know this code at all, so I'm not sure where things are going wrong here.  Is it not expected that the listener is capturing the window, or should the listener be uninstalled when the installation is cancelled?

I tried to avoid the former problem by hoisting the callback definitions outside of AMC_installAddonsFromWebpage, but that just broke addon installation.  I'm not sure why.

(By the way, anybody should feel free to steal this bug from me.)

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: my crude attempt to avoid capturing aWindow by hoisting the definition of the callbacks

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: avoid capturing aWindow by hoisting the definition of callbacks

This one doesn't break the installer.  It also removes the leaks via aWindow, but there are still other things keeping the compartment alive.  The paths from those objects to the other compartment mostly look similar to this:

via Preserved wrapper :
0x120e49558 [XULElement 1469762b0]
    0x117a3e000 --[notification]-> 0x120a444a0 [Object <no private>]
    0x117a3e000 --[options]-> 0x120a44160 [Object <no private>]
    0x117a3e000 --[installs]-> 0x120a68fa8 [Array 1]
    0x117a3e000 --[element[0]]-> 0x120ae3cc0 [Object <no private>]
    0x117a3e000 --[addListener]-> 0x120ea1b30 [Function 118332c80]
    0x117a3e000 --[upvars[0]]-> 0x120ae3710 [Object <no private>]
    0x117a3e000 --[window]-> 0x119f153d0 [Proxy <no private>]
    0x117a3e000 --[extra0]-> 0x120e46f98 [NativePropertyHolder <no private>]
    0x117a3e000 --[location]-> 0x119f819e8 [Proxy <no private>]
    0x117a3e000 --[type]-> 0x120e66dc0 [type_object [0x120e66dc0]]
    0x117a3e000 --[type_proto]-> 0x119f81980 [Proxy <no private>]
    0x117a3e000 --[type]-> 0x120ecae00 [type_object [0x120ecae00]]
    0x117a3e000 --[type_proto]-> 0x120a23f98 [Proxy <no private>]

Note that the options, installs, element[0], addListener segment is similar to before.  Maybe installs or the array of listeners isn't being cleared when it should be?

Comment 13

[Andrew McCreight [:mccr8]]

I think I've gotten about as far as I can get without knowing any of this code.  If anybody wants to look into this, I can give them some of my log files or whatnot.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

I wonder whether this might be related to bug 669240.

Comment 15

[Andrew McCreight [:mccr8]]

P3 because people don't install addons that often.

Comment 16

[Nicholas Nethercote [inactive]]

I can't reproduce this using the steps from comment 0 or comment 6, or several variations on them.  Please reopen if anyone else can.