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
Closed

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

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

Comments

@juancampa
Copy link

juancampa 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.

@sindresorhus
Copy link
Owner

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

@greghawk
Copy link

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

@keithamus
Copy link
Contributor

keithamus 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
Copy link
Collaborator

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
Copy link
Collaborator

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

sigv added a commit to sigv/mem that referenced this issue Jul 17, 2019
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
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
Development

Successfully merging a pull request may close this issue.

5 participants