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

Throw an error if users try to initialise LocalStorage or global instance more than once #150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gpoole
Copy link

@gpoole gpoole commented Feb 7, 2024

This change throws an error if a single instance of LocalStorage or the global instance is initialised more than once. This helps users avoid accidentally initialising multiple times in both cases, which I believe is not intended to be supported and, for the global instance, causes a resource leak.

Since this change means node-persist will throw exceptions in places where it previously didn't I think this is a breaking change.

We had a problem in our app where CPU usage would slowly climb over time and eventually lock up the server. We found the global node-persist init was being called in a function that was itself being called many times per request. While LocalStorage seems to be ok with init being called multiple times (but IMO shouldn't silently allow it), the global init creates a new LocalStorage instance every time it's called. If either write queues or expiry are enabled (they are by default), the new instances keep registering new interval callbacks and the old ones are not cancelled, preventing the old instances from being garbage collected and forcing node to process an increasing number of callbacks over time until eventually all CPU is consumed.

@gpoole
Copy link
Author

gpoole commented Feb 7, 2024

This PR doesn't really help with users accidentally creating lots of their own instances of LocalStorage and not cleaning up. I think probably too hard to detect? Maybe a destroy method that calls stopExpiredKeysInterval and stopWriteQueueInterval and some docs on needing to use it?

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

Successfully merging this pull request may close these issues.

1 participant