node-persist icon indicating copy to clipboard operation
node-persist copied to clipboard

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

Open gpoole opened this issue 1 year ago • 1 comments

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 avatar Feb 07 '24 01:02 gpoole

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?

gpoole avatar Feb 07 '24 02:02 gpoole