847771 - Panels leak memory after unload

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Erik Vold [:erikvold] (please needinfo? me)]

I saw https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/frame/hidden-frame.js#L168-L169 today and thought to check if the Panel module was leaking, and it appears to be..  I'm not sure if the link above is the cause or not right now though.

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

It looks like this file was messed up by https://github.com/mozilla/addon-sdk/pull/698 and the revert was not done properly.

https://github.com/mozilla/addon-sdk/commit/670279994b4c49cc71960c022165b96de41f0d2e#lib/sdk/frame/hidden-frame.js

I think that we should restore the Panel related code to the way it was, and not make updates to it until the pwpb changes have been made as I requested here https://github.com/mozilla/addon-sdk/pull/698#issuecomment-12450522

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Hey Erik I just looked at the diff of the merge:
https://github.com/mozilla/addon-sdk/commit/42cede06862471f3c05cb937cb3d2200b4d3218e

Diff-stat: 7 changed files with 115 additions and 86 deletions. 

And at the reverting commit:
https://github.com/mozilla/addon-sdk/commit/670279994b4c49cc71960c022165b96de41f0d2e

Diff-stat: 7 changed files with 86 additions and 115 deletions.

So I believe that code was restored to it's previous state. What makes you believe it wasn't ? I also remember diffing reverted change to the state just before my original commit and delta was empty.

Can you provide little more info about why you think panel leaks and how did you test that ?

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

Ahh it looks like github has a bug, it's showing the contents on master no matter what commit I choose..

This leak was introduced much further back sadly..

https://github.com/Gozala/addon-sdk/commit/58aabad258ea6544a6a61ca2c3449b749a81b9e0

https://github.com/mozilla/addon-sdk/pull/668

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

My mistake this is the one:

https://github.com/Gozala/addon-sdk/commit/a340d1753cc5f5276f7b64f1063bca6f43eb7969

https://github.com/mozilla/addon-sdk/pull/695

https://github.com/Gozala/addon-sdk/commit/a340d1753cc5f5276f7b64f1063bca6f43eb7969#L1R168

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

Ok this isn't in 1.13.2, but it is on stabilization

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/874

Pointer to Github pull-request

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 726995 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/874

This is just a typo fix against stabilization, I'll leave up to you to decide weather it's worth landing and uplifting.

I'll write tests against master and submit separate pull request for it.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 726995 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/874

Seems easy and harmless enough to me...

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/875

Pointer to Github pull-request

Comment 10

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 727020 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/875

Fix for the master + tests.

Comment 11

[[github robot]]

Commit pushed to stabilization at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/67dfdb63ae01d7c619cfab42ce33301561acfcee
Merge pull request #874 from Gozala/hotfix/panel-leak

Bug 847771 - Hotfix typo in hidden-frame module. r=@erikvold

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/1177cc3de3aee511d228c201ae07057e7f824cdc
Merge pull request #875 from Gozala/bug/panel-leaks@847771

Bug 847771 - Fix memory leaks and add tests r=@erikvold