The default bug view has changed. See this FAQ.

UI Enhancer add-on causes immortal zombie compartments

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: TheOne, Assigned: Optimizer)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink], URL)

Attachments

(1 attachment, 3 obsolete attachments)

96.32 KB, application/octet-stream
TheOne
: review+
Details
(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
Assignee: general → scrapmachines
Summary: UI enhancer causes immortal zombie compartments → UI Enhancer add-on causes immortal zombie compartments

Comment 2

5 years ago
Created attachment 594134 [details] [diff] [review]
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

5 years ago
Created attachment 594135 [details]
Patched XPI, v0
(Reporter)

Comment 4

5 years ago
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/UIEnhancer@girishsharma.xpi!/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
(Assignee)

Comment 5

5 years ago
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

5 years ago
Oops, forgot to look after the bootstrap compartment.
My patch should address the leaky inner-windows/content compartments, though
(Assignee)

Comment 7

5 years ago
Created attachment 594210 [details]
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.
Attachment #594210 - Flags: review?(mail)

Comment 8

5 years ago
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)
Attachment #594210 - Flags: review-
(Assignee)

Comment 9

5 years ago
(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.
(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
(Assignee)

Comment 11

5 years ago
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 ?
(Reporter)

Comment 12

5 years ago
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.
(Assignee)

Updated

5 years ago
Attachment #594210 - Attachment is obsolete: true
Attachment #594210 - Flags: review?(mail)
(Assignee)

Comment 13

5 years ago
Created attachment 594231 [details]
Fix 2

Merged patch v0.
Attachment #594231 - Flags: review?(mail)
(Reporter)

Comment 14

5 years ago
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.
Attachment #594231 - Flags: review?(mail) → review+
(Reporter)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Attachment #594134 - Attachment is obsolete: true
(Reporter)

Updated

5 years ago
Attachment #594135 - Attachment is obsolete: true
(Assignee)

Comment 15

5 years ago
Yes I know, but I don't have a mac to properly debug this issue :(
Will try my best.
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. ;)
Version 3.6 released and available from AMO
Status: RESOLVED → VERIFIED
I can't find this add-on on AMO now.  The URL given above is a 404.
(Assignee)

Comment 19

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

Here you go.
(Assignee)

Updated

5 years ago
You need to log in before you can comment on or make changes to this bug.