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

should remove part of memory leaks closures for callback in event listen... #282

Closed
wants to merge 2 commits into from

Conversation

@arestov
Copy link

arestov commented Mar 18, 2015

...ers in foreight objects

@feross

This comment has been minimized.

Copy link
Member

feross commented Mar 18, 2015

Thanks for the PR. How did you measure that these changes actually reduce memory usage? How can I confirm on my machine?

@arestov arestov force-pushed the arestov:master branch 4 times, most recently from 2ef7cc9 to cd22aa1 Mar 19, 2015
@arestov

This comment has been minimized.

Copy link
Author

arestov commented Mar 19, 2015

I did not make any mesures for this.

This case https://cloud.githubusercontent.com/assets/69734/5448999/74155c90-8505-11e4-9dd4-afbf7cb99ce6.png revealed to me that code has not any unsubscribtion for events.

But them should be because it common case for memory leaks
https://msdn.microsoft.com/en-us/library/ie/dn741342(v=vs.94).aspx
(dev placed some object in closure, function with closure are placed in
_events = {
eventName: [func]
} cache. Thats all - now you have to remove this reference chain if you want to remove object from memory

So this should fix not only part of confirmed leaks, but also just potential leaks

@arestov arestov closed this Mar 21, 2015
@feross

This comment has been minimized.

Copy link
Member

feross commented Mar 22, 2015

Why did you close the issue?

@feross feross reopened this Mar 22, 2015
@arestov

This comment has been minimized.

Copy link
Author

arestov commented Mar 23, 2015

I commited from fork master where I plan to make more changes. (so i did) I want to make new pull request from other branch (wich I still did not make)

@arestov arestov force-pushed the arestov:master branch from e582e1e to 30db7ae Apr 21, 2015
@arestov arestov force-pushed the arestov:master branch from 30db7ae to 123dfe9 Jul 19, 2015
@arestov

This comment has been minimized.

Copy link
Author

arestov commented Jul 20, 2015

@arestov arestov force-pushed the arestov:master branch 4 times, most recently from d68650f to eb2d4ab Jul 21, 2015
@arestov arestov force-pushed the arestov:master branch 3 times, most recently from afc7059 to ee46b42 Jan 8, 2016
@arestov arestov mentioned this pull request Mar 30, 2016
arestov added 2 commits Jul 20, 2015
…teners in foreight objects
@arestov arestov force-pushed the arestov:master branch from ee46b42 to 79e401c Mar 31, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.