Last Comment Bug 1001833 - "TypeError: can't access dead object" due leaks of event/dom and event/chrome modules
: "TypeError: can't access dead object" due leaks of event/dom and event/chrome...
Status: RESOLVED FIXED
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
P1 normal (vote)
: ---
Assigned To: Matteo Ferretti [:zer0] [:matteo]
:
:
Mentors:
: 1009193 (view as bug list)
Depends on:
Blocks: 973641
  Show dependency treegraph
 
Reported: 2014-04-26 05:14 PDT by Matteo Ferretti [:zer0] [:matteo]
Modified: 2014-07-02 15:21 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
pull/1476 (46 bytes, text/x-github-pull-request)
2014-04-27 00:06 PDT, Matteo Ferretti [:zer0] [:matteo]
jordan: review+
Details | Review | Splinter Review

Description User image Matteo Ferretti [:zer0] [:matteo] 2014-04-26 05:14:08 PDT
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.
Comment 1 User image Matteo Ferretti [:zer0] [:matteo] 2014-04-27 00:00:30 PDT
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.
Comment 2 User image Matteo Ferretti [:zer0] [:matteo] 2014-04-27 00:06:58 PDT
Created attachment 8413326 [details] [review]
pull/1476
Comment 3 User image Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2014-04-29 10:54:56 PDT
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.
Comment 4 User image Matteo Ferretti [:zer0] [:matteo] 2014-05-13 09:31:51 PDT
*** Bug 1009193 has been marked as a duplicate of this bug. ***
Comment 5 User image Alan 2014-05-27 16:05:44 PDT
I'm experiencing this bug on x86 windows as well.
Comment 6 User image Anthony Hughes (:ashughes) [QA] 2014-06-11 08:10:10 PDT
Is there any update on this bug? It's blocking some of our Australis automation work.
Comment 7 User image Matteo Ferretti [:zer0] [:matteo] 2014-06-30 11:30:33 PDT
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.
Comment 8 User image Jordan Santell [:jsantell] [@jsantell] 2014-07-01 09:55:08 PDT
Added some comments, anyway we can test this? Seems like some of the logic is reversed, too
Comment 9 User image Matteo Ferretti [:zer0] [:matteo] 2014-07-01 19:01:35 PDT
(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.
Comment 10 User image [github robot] 2014-07-02 15:21:27 PDT
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

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