"TypeError: can't access dead object" due leaks of event/dom and event/chrome modules

RESOLVED FIXED

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: zer0, Assigned: zer0)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The ActionButton and ToggleButton seems have a leak somewhere; probably there is a clean up that is not made properly. Just have an add-on with only the following line:

  require("sdk/ui/button/action");

Once disabled, clicks to any button on the toolbar (e.g. Home Button) and the console will log "JavaScript error: , line 0: can't access dead object".

It seems a listener that is still alive, probably related to CustomizableUI. I'm working on it.
(Assignee)

Updated

4 years ago
Assignee: nobody → zer0
(Assignee)

Updated

4 years ago
Blocks: 973641
(Assignee)

Comment 1

4 years ago
After some investigation, it seems that the `open` function of `event/dom` module is the culprit: it's adding listener to a DOM element – in our case, a window - without remove them once the add-on is unloaded.
Removing those listeners during the unload of the add-on, fixes the issue.
Summary: "TypeError: can't access dead object" with Button APIs → "TypeError: can't access dead object" due leaks of event/dom module
(Assignee)

Comment 2

4 years ago
Created attachment 8413326 [details] [review]
pull/1476
Attachment #8413326 - Flags: review?(rFobic)
Comment on attachment 8413326 [details] [review]
pull/1476

Denying the review because it will likely introduce flip side issue where dead object errors will be logged on add-on unload for windows that have being GC-ed.
Attachment #8413326 - Flags: review?(rFobic) → review-

Updated

4 years ago
Priority: -- → P1
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1009193

Comment 5

4 years ago
I'm experiencing this bug on x86 windows as well.
OS: Mac OS X → All
Hardware: x86 → All
Is there any update on this bug? It's blocking some of our Australis automation work.
(Assignee)

Updated

4 years ago
Summary: "TypeError: can't access dead object" due leaks of event/dom module → "TypeError: can't access dead object" due leaks of event/dom and event/chrome modules
(Assignee)

Comment 7

4 years ago
Comment on attachment 8413326 [details] [review]
pull/1476

Assign the new review to someone else 'cause Irakli has days off.

Notice, for the reviewer:

> Denying the review because it will likely introduce flip side issue 
> where dead object errors will be logged on add-on unload for windows
> that have being GC-ed.

That wasn't actually true, the "dead object" exception was generated by the event/chrome module, that the previous patch didn't considered.
However, the previous code didn't raise "dead object" exception in such case only because a memory leak: the `window` reference was kept alive even if the window itself was closed. This patch trying to avoid this memory leaks as well.
Attachment #8413326 - Flags: review- → review?(jsantell)
Added some comments, anyway we can test this? Seems like some of the logic is reversed, too
(Assignee)

Comment 9

4 years ago
(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #8)
> Added some comments, anyway we can test this? Seems like some of the logic
> is reversed, too

I fixed the reversed logic, and also added a test, that I checked both with and without the fix. I'm not sure if it's the best way to test it, I feel like the `nuke` part could be maybe dangerous to use.
BTW, ready for the second round of review.
Flags: needinfo?(jsantell)
Attachment #8413326 - Flags: review?(jsantell) → review+
Flags: needinfo?(jsantell)

Comment 10

4 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/21496d5835ec5c1d65bcb24fad57f6c20ea6db40
Bug 1001833 - "TypeError: can't access dead object" due leaks of event/dom and event/chrome modules

- Ensured listeners and observers are removed once the add-on is disabled / uninstalled

https://github.com/mozilla/addon-sdk/commit/461e57c8221aaab907e56c9e9de131629e8ae39c
Merge pull request #1476 from ZER0/dead-object/1001833

fix Bug 1001833 - "TypeError: can't access dead object" due leaks of event/dom module r=@jsantell

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.