hookstate icon indicating copy to clipboard operation
hookstate copied to clipboard

Fix subscribers memory leak

Open speigg opened this issue 7 months ago • 2 comments

Memory Leak Fix

Problem

There was a memory leak in the hookstate library where states don't properly unsubscribe on store switchovers, introduced in PR #409. The issue occurs when incrementally switching to new global stores.

Solution

We fixed the memory leak by:

  1. Adding an internal cleanupOrphanedSubscribers method to the Store class that cleans up subscribers that are no longer mounted:
// Internal method to clean up orphaned subscribers
// This is the key fix for the memory leak
cleanupOrphanedSubscribers() {
    if (this._subscribers.size > 1) {
        // Create a new set to avoid modification during iteration
        const subscribers = new Set(this._subscribers);
        subscribers.forEach(subscriber => {
            // Always keep the root state subscriber
            if (subscriber === this._stateMethods) {
                return;
            }

            // Only remove subscribers that are unmounted
            if (subscriber instanceof StateMethodsImpl &&
                !subscriber.isMounted) {
                this.unsubscribe(subscriber);
            }
        });
    }
}
  1. Modifying the useHookstate function to mark the old state as unmounted BEFORE creating a new one:
if (value.store !== parentMethods.store || !('source' in value)) {
    // Get a reference to the old store before we create a new one
    const oldStore = value.store;
    const oldState = value.state;

    // Mark the old state as unmounted BEFORE creating a new one
    // This is the key fix for the memory leak
    oldState.onUnmount();

    // Create a new value with the new store
    value = initializer();

    // Now properly clean up the old store
    oldStore.unsubscribe(oldState);

    // Force cleanup of orphaned subscribers
    // This is needed because the store keeps a reference to the state
    // even after the state is unmounted
    oldStore.cleanupOrphanedSubscribers();
}
  1. Calling cleanupOrphanedSubscribers in the unmount handler to ensure all orphaned subscribers are properly cleaned up when the component unmounts:
return () => {
    // This is the key fix for the memory leak
    // We need to properly clean up when the component unmounts
    value.state.onUnmount()
    value.store.unsubscribe(value.state);

    // Force cleanup of the store's subscribers
    // This is needed because the store keeps a reference to the state
    // even after the state is unmounted
    value.store.cleanupOrphanedSubscribers();

    // Deactivate the store which will clean up all subscribers
    value.store.deactivate() // this will destroy the extensions
}

Key Improvements

  1. The cleanup is selective - it only removes subscribers that are unmounted, rather than all subscribers. This ensures that legitimate subscribers remain active.

  2. The cleanup happens automatically as part of the normal component lifecycle.

speigg avatar Apr 29 '25 03:04 speigg

Hi, why do you need to _ prefix method when it can be simply non public method enforced by typescript?

avkonst avatar Apr 29 '25 06:04 avkonst

Oh, good point, I'll unprefix :)

speigg avatar Apr 29 '25 15:04 speigg