723843 - UI Enhancer add-on causes immortal zombie compartments

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Wagner [:TheOne] [use NI]]

STR:

1) Install the add-on from the url above
2) Close all tabs and go to about:memory?verbose
3) Disable add-on, close the add-ons manager tab
4) Click <minimize memory usage>

I also tried even closing the about:memory tab after disabling the add-on and opening it in a new one. Same result.

Comment 1

[Girish Sharma [:Optimizer]]

I am trying to get rid of this problem, is this totally the add-on's problem? or Firefox's also ?
Please assign this to me.

Comment 2

[Nils Maier [:nmaier]]

Attachment: non-author patch, v0

The add-on also leaks without disabling it first.

After applying the above patch I cannot seem to reproduce the leaks any longer, on Nightly at least.
Andreas and Girish, can you verify the problem is gone with the patch?

Comment 3

[Nils Maier [:nmaier]]

Attachment: Patched XPI, v0

Comment 4

[Andreas Wagner [:TheOne] [use NI]]

Nils, I cannot confirm that this is resolved in your patched XPI (tested on Firefox 11.0a2):

After disabling the add-on and clicking the <minimize memory usage> button (and waiting some time and clicking it again), I still see the following branch in about:memory:

├─────360,976 B (00.92%) -- compartment([System Principal], jar:file:///C:/FirefoxProfiles/develop_4.0/extensions/[e-mail]!/bootstrap.js, 0x95c0000)
│  │     ├──184,320 B (00.47%) -- gc-heap
│  │     │  ├───87,352 B (00.22%) -- arena
│  │     │  │   ├──85,304 B (00.22%) -- unused
│  │     │  │   ├───1,328 B (00.00%) -- padding
│  │     │  │   └─────720 B (00.00%) -- headers
│  │     │  ├───40,032 B (00.10%) -- objects
│  │     │  │   ├──20,480 B (00.05%) -- non-function
│  │     │  │   └──19,552 B (00.05%) -- function
│  │     │  ├───30,464 B (00.08%) -- scripts
│  │     │  ├───25,256 B (00.06%) -- shapes
│  │     │  │   ├──18,888 B (00.05%) -- tree
│  │     │  │   ├───3,392 B (00.01%) -- base
│  │     │  │   └───2,976 B (00.01%) -- dict
│  │     │  ├────1,184 B (00.00%) -- type-objects
│  │     │  └───────32 B (00.00%) -- strings
│  │     ├───86,008 B (00.22%) -- script-data
│  │     ├───65,536 B (00.17%) -- mjit-code
│  │     ├───15,368 B (00.04%) -- shapes-extra
│  │     │   ├───7,176 B (00.02%) -- compartment-tables
│  │     │   ├───3,904 B (00.01%) -- tree-tables
│  │     │   ├───3,072 B (00.01%) -- tree-shape-kids
│  │     │   └───1,216 B (00.00%) -- dict-tables
│  │     ├────8,224 B (00.02%) -- object-slots
│  │     ├────1,248 B (00.00%) -- type-inference
│  │     │    └──1,248 B (00.00%) -- script-main
│  │     └──────272 B (00.00%) -- string-chars

Comment 5

[Girish Sharma [:Optimizer]]

No help.
With your patch applied, after uninstalling the add-on, the add-on still consumes around 700 KB space, that is a zombie compartment.

The problem is caused due to non-null DOM references which still remain after uninstalling. I am looking into it and will update soon.

Comment 6

[Nils Maier [:nmaier]]

Oops, forgot to look after the bootstrap compartment.
My patch should address the leaky inner-windows/content compartments, though

Comment 7

[Girish Sharma [:Optimizer]]

Attachment: fix 1

It seems that the async function that I have been using for delayed call to a function was leaking memory.
Fixed and updated in the xpi attached.

Comment 8

[Nils Maier [:nmaier]]

Comment on attachment 594210 [details]
fix 1

Leaks content compartments for me:

STR:
- Have UIEnh enabled
- Open http://google.de/ in a tab
- Close tab
- Check about:memory after minimizing for google.de compartments/windows
-> Still present and hence leaked
- Disable UIEnh
-> All gone now (so that leak is fixed)

Comment 9

[Girish Sharma [:Optimizer]]

(In reply to Nils Maier [:nmaier] from comment #8)
> Leaks content compartments for me:
> 
> STR:
> - Have UIEnh enabled
> - Open http://google.de/ in a tab
> - Close tab
> - Check about:memory after minimizing for google.de compartments/windows
> -> Still present and hence leaked
> - Disable UIEnh
> -> All gone now (so that leak is fixed)
Did your patch v0 earlier address this issue ?
My patch only addresses the zombie compartment formed after uninstalling/disabling the add-on.

Comment 10

[Nils Maier [:nmaier]]

(In reply to scrapmachines from comment #9)
> (In reply to Nils Maier [:nmaier] from comment #8)
> Did your patch v0 earlier address this issue ?
> My patch only addresses the zombie compartment formed after
> uninstalling/disabling the add-on.

It does, indeed. Patched my v0 into your fix1 and all leaks are gone.

I did glance over:
https://github.com/scrapmac/UIEnhancer/commit/646fed82c3c3ac25632ecc564dc29f35fc6360d9#L0R2166
Thought this was my fix already, but in fact it isn't

But there is a new issue, unrelated to the leaks.
STR
- Open some URL, e.g about about:memory?verbose
- Click/focus into the urlbar so that it will be in "edit mode"
- Leave the urlbar (click somewhere else)
-> The urlbar is left empty

Comment 11

[Girish Sharma [:Optimizer]]

Yes I know of that issue, I will upload the fixed xpi on AMO in few minutes (including your patch).

Is it now good to set this bug to resolved fixed ?

Comment 12

[Andreas Wagner [:TheOne] [use NI]]

Well, you could set it to fixed but we will reopen if the add-on is still leaking in our tests.

Maybe you want to upload it here to bugzilla first, so we can test it before you upload it to amo.

Comment 13

[Girish Sharma [:Optimizer]]

Attachment: Fix 2

Merged patch v0.

Comment 14

[Andreas Wagner [:TheOne] [use NI]]

Comment on attachment 594231 [details]
Fix 2

Yes, that works fine. Thanks you!

Just as a note: Another lightly unrelated issue is that the height of the urlbar changes depending on whether it is focused or not. This makes the elements below the urlbar "jump" up and down which looks quite ugly.
This won't prevent you from getting full approval, but while you're at fixing stuff, you might wanna fix this too, for the sake of your Mac users :)

Thanks.

Comment 15

[Girish Sharma [:Optimizer]]

Yes I know, but I don't have a mac to properly debug this issue :(
Will try my best.

Comment 16

[Nils Maier [:nmaier]]

The review queues are really fast right now.
I suggest you first upload your fixed version and address any Mac layout issues in subsequent releases. ;)

Comment 17

[Nils Maier [:nmaier]]

Version 3.6 released and available from AMO

Comment 18

[Nicholas Nethercote [inactive]]

I can't find this add-on on AMO now.  The URL given above is a 404.

Comment 19

[Girish Sharma [:Optimizer]]

https://addons.mozilla.org/en-US/firefox/addon/ui-enhancer/

Here you go.