Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX lts] Remove all own listeners #18150

Merged
merged 1 commit into from
Jun 26, 2019
Merged

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Jun 26, 2019

We were previously only removing own listeners if they were functions,
this PR updates us to always remove all of them.

Fixes #18140

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seem generally good, but we really need to get a regression test in here too I think...

@pzuraq pzuraq force-pushed the bugfix/remove-all-own-listeners branch from a424258 to 80fddf2 Compare June 26, 2019 19:32
We were previously only removing own listeners if they were functions,
this PR updates us to always remove all of them.
@pzuraq pzuraq force-pushed the bugfix/remove-all-own-listeners branch from 80fddf2 to 96a832b Compare June 26, 2019 19:32
@rwjblue rwjblue merged commit 93ef080 into master Jun 26, 2019
@rwjblue
Copy link
Member

rwjblue commented Jun 26, 2019

Thank you!

@rwjblue rwjblue deleted the bugfix/remove-all-own-listeners branch June 26, 2019 22:25
@rwjblue rwjblue added the Bug label Jun 26, 2019
@BillyRayPreachersSon
Copy link

BillyRayPreachersSon commented Jun 27, 2019

Will this fix be backported to Ember 3.8, so that LTS users get the benefit? This is really hampering us upgrading from 3.4 LTS, as the memory usage in our app goes through the roof (in one test alone, the retained memory goes from 1.5MB under Ember 3.4 to around 35MB under Ember 3.8).

For our Ember setup (ember-cli 3.8.3 with ember-source 3.8.2), removing the function check from ember.prod.js and ember.debug.js in our node_modules/ember-source/dist/ and node_modules/ember-source/dist/legacy/ folders fixes the memory leaks in the tests, so it's definitely looking good for us.

Thanks!

@pzuraq
Copy link
Contributor Author

pzuraq commented Jun 27, 2019

The fix is marked for LTS releases, so yes, it will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory issues in 3.8
3 participants