simonlast / node-persist

Super-easy persistent data structures in Node.js
MIT License
719 stars 78 forks source link

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

Open gpoole opened 9 months ago

gpoole commented 9 months ago

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 commented 9 months ago

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?