-
Notifications
You must be signed in to change notification settings - Fork 89
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
AsyncHook MemoryLeak fixes #60
base: master
Are you sure you want to change the base?
Conversation
* in case our namespace is retained mistakenly, so | ||
* we releasing heavist part at least | ||
*/ | ||
namespace._contexts = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of typos, and this shouldn't be JSDoc block. Maybe:
/*
* Zeroing _contexts as heaviest part of Namespace
* In case our namespace is retained mistakenly, so
* at least we are releasing heaviest part
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with these changes. Thanks for suggestions.
@Jeff-Lewis ping( |
Any updates on this @Jeff-Lewis ? |
You haven't yet noticed that cls-hooked has a major memory leak? See: And btw: a performance problem as well? |
It looks like we have to
disable
hook, when namespace is destoryed. Otherwise it would live forever and hence it would retain namespace object with all it's massive _context.More details how I replicated and fixed the issue described here: liberation-data/drivine#35 (comment)