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

Memory leak: old results are not deleted from the cache #14

Closed
juancampa opened this issue Jan 17, 2018 · 5 comments · Fixed by #19

Comments

@juancampa
Copy link

commented Jan 17, 2018

Looking at the code it seems like old results are never removed from the cache, even when they reach their maxAge and become forever unused. I might be misreading it though.

@lukechilds lukechilds referenced this issue Feb 5, 2018
keithamus added a commit to keithamus/mem that referenced this issue Feb 21, 2018
keithamus added a commit to keithamus/mem that referenced this issue Feb 21, 2018
keithamus added a commit to keithamus/mem that referenced this issue Feb 21, 2018
@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Jun 20, 2018

Even if you can't submit a fix, submitting a pull request with a failing test would be very helpful.

@greghawk

This comment has been minimized.

Copy link

commented Jul 19, 2018

@sindresorhus @keithamus Is there a fix for this? it is blowing out my servers memory at volume

@keithamus

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

@greghawk for now you have to pass in your own Map as the {cache} option - and then using a timer of your own, empty the Map.

I'll add to this that I don't think this should be solved by mem alone. I think adding timeouts to mem would add too much internal state. I think the solution is perhaps to write a ttlMap style lib that complements this lib. (or of course, as the README suggests, you can use quick-lru)

@SamVerschueren

This comment has been minimized.

Copy link
Collaborator

commented Aug 17, 2018

I'm implementing it as we speak for expiry-map as it was reported as an issue over there as well.

Because I also need it for expiry-set, I'm looking in extracting the logic in a separate package. Still have some cleanup work to do and then actually extracting and releasing it as a package. But I believe it can easily be used by mem.

@SamVerschueren

This comment has been minimized.

Copy link
Collaborator

commented Aug 22, 2018

I implemented this in #19. Feel free to help testing and reviewing it.

SamVerschueren added a commit that referenced this issue Aug 27, 2018
sindresorhus added a commit that referenced this issue Aug 27, 2018
sigv added a commit to sigv/mem that referenced this issue Jul 17, 2019
Backport memory leak fixes
Commit da4e439 resolved memory
leaking issues by introducing a dependency that was not compatible
with Node.js v4 which mem v1 supported. This commit adds timers
for ensuring the cache Map is cleaned up without using additional
dependencies.

Bug ref: sindresorhus#14
sigv added a commit to sigv/mem that referenced this issue Jul 17, 2019
Backport memory leak fixes
Commit da4e439 resolved memory
leaking issues by introducing a dependency that was not compatible
with Node.js v4 which mem v1 supported. This commit adds timers
for ensuring the cache Map is cleaned up without using additional
dependencies.

Bug ref: sindresorhus#14

Repository owner locked and limited conversation to collaborators Jul 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.