From a3fb0d910f4cdf5ca45d4c8348838486fb4828b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20R=C3=A9mond?= Date: Fri, 13 Feb 2026 08:48:25 +0100 Subject: [PATCH 1/3] fix(sdk): prevent memory and CPU leaks on connection loss Store and call cleanup functions for store subscriptions and side effects in XMPPClient.destroy(). Previously, these unsubscribe functions were discarded, causing subscriptions to accumulate and leak memory/CPU when connections were lost or intermittent. Changes: - Store cleanup functions from setupPresenceSync, createStoreBindings, and setupStoreSideEffects in XMPPClient - Call all cleanup functions in destroy() method - Add MUC.cleanup() to clear pending join timeouts on disconnect - Clear typing timeouts when going offline to prevent orphaned timers --- packages/fluux-sdk/src/core/XMPPClient.ts | 58 +++++++++++----------- packages/fluux-sdk/src/core/modules/MUC.ts | 14 ++++++ packages/fluux-sdk/src/core/sideEffects.ts | 10 +++- 3 files changed, 51 insertions(+), 31 deletions(-) diff --git a/packages/fluux-sdk/src/core/XMPPClient.ts b/packages/fluux-sdk/src/core/XMPPClient.ts index 9079364..77113a1 100644 --- a/packages/fluux-sdk/src/core/XMPPClient.ts +++ b/packages/fluux-sdk/src/core/XMPPClient.ts @@ -276,6 +276,13 @@ export class XMPPClient { */ private modulesInitialized = false + /** + * Cleanup functions for store subscriptions. + * Called in destroy() to prevent memory leaks. + * @internal + */ + private cleanupFunctions: (() => void)[] = [] + /** * Creates a new XMPPClient instance. * @@ -333,9 +340,10 @@ export class XMPPClient { }).start() // Subscribe to persist state changes to sessionStorage - this.presenceActor.subscribe(() => { + const unsubscribePresencePersist = this.presenceActor.subscribe(() => { savePresenceSnapshot(this.presenceActor) }) + this.cleanupFunctions.push(unsubscribePresencePersist) // Create presence options that read from the machine (single source of truth) const presenceOptions: DefaultStoreBindingsOptions = { @@ -384,18 +392,12 @@ export class XMPPClient { this.initializeModules(createDefaultStoreBindings(presenceOptions)) // Set up presence sync (machine state -> XMPP presence) - // The subscription is permanent for the lifetime of the client. - // We don't store the unsubscribe function because in React StrictMode, - // destroy() is called between mount cycles but the client persists. - this.setupPresenceSync(this.presenceActor) + const unsubscribePresenceSync = this.setupPresenceSync(this.presenceActor) + this.cleanupFunctions.push(unsubscribePresenceSync) // Set up SDK event -> Zustand store bindings // This wires SDK events (e.g., 'chat:message') to store updates (e.g., chatStore.addMessage) - // Note: We don't store the unsubscribe function because: - // 1. Bindings are permanent for the lifetime of the client - // 2. In React StrictMode, destroy() is called between mount cycles but the client persists - // 3. Bindings are garbage collected when the client is - createStoreBindings(this, () => ({ + const unsubscribeStoreBindings = createStoreBindings(this, () => ({ connection: connectionStore.getState(), chat: chatStore.getState(), roster: rosterStore.getState(), @@ -405,13 +407,11 @@ export class XMPPClient { blocking: blockingStore.getState(), console: consoleStore.getState(), })) + this.cleanupFunctions.push(unsubscribeStoreBindings) // Set up store-based side effects (activeConversation -> load cache, MAM fetch) - // Note: Like store bindings, we don't store the unsubscribe function because: - // 1. Side effects are permanent for the lifetime of the client - // 2. In React StrictMode, destroy() is called between mount cycles but the client persists - // 3. Subscriptions are garbage collected when the client is - setupStoreSideEffects(this) + const unsubscribeSideEffects = setupStoreSideEffects(this) + this.cleanupFunctions.push(unsubscribeSideEffects) } /** @@ -886,21 +886,19 @@ export class XMPPClient { * ``` */ destroy(): void { - // NOTE: We intentionally do NOT clean up store bindings, presence sync, - // or the presence actor here. - // - // All of these are created once in the constructor and persist for the - // lifetime of the client. In React StrictMode, useEffect cleanup runs - // between mount cycles, but the client ref persists. If we cleaned up - // these resources here, they couldn't be recreated without making a new - // client, which would break presence state. - // - // Specifically: - // - Store bindings wire SDK events to Zustand stores - // - Presence sync subscription sends XMPP presence on machine state changes - // - Presence actor manages presence state machine - // - // All resources will be garbage collected when the client itself is. + // Clean up all store subscriptions and side effects to prevent memory leaks. + // This is called when XMPPProvider unmounts or when the client is destroyed. + for (const cleanup of this.cleanupFunctions) { + try { + cleanup() + } catch { + // Ignore cleanup errors + } + } + this.cleanupFunctions = [] + + // Clean up MUC pending joins to prevent orphaned timeouts + this.muc?.cleanup() } /** diff --git a/packages/fluux-sdk/src/core/modules/MUC.ts b/packages/fluux-sdk/src/core/modules/MUC.ts index 0a40f34..0695ed2 100644 --- a/packages/fluux-sdk/src/core/modules/MUC.ts +++ b/packages/fluux-sdk/src/core/modules/MUC.ts @@ -270,6 +270,20 @@ export class MUC extends BaseModule { } } + /** + * Clean up all pending operations. + * Called when the client is destroyed or connection is lost to prevent + * memory leaks from orphaned timeouts. + */ + cleanup(): void { + // Clear all pending join timeouts + for (const pending of this.pendingJoins.values()) { + clearTimeout(pending.timeoutId) + } + this.pendingJoins.clear() + this.pendingOccupants.clear() + } + /** * Flush buffered occupants for a room in a single batch update. * This reduces store updates from ~N to 1 for large rooms during join. diff --git a/packages/fluux-sdk/src/core/sideEffects.ts b/packages/fluux-sdk/src/core/sideEffects.ts index 3289b14..05fddd7 100644 --- a/packages/fluux-sdk/src/core/sideEffects.ts +++ b/packages/fluux-sdk/src/core/sideEffects.ts @@ -182,11 +182,19 @@ export function setupChatSideEffects( { fireImmediately: false } ) - // Subscribe to connection status changes (for reconnection catch-up) + // Subscribe to connection status changes (for reconnection catch-up and cleanup) // Note: connectionStore doesn't use subscribeWithSelector, so we track previous status manually let previousStatus = connectionStore.getState().status const unsubscribeConnection = connectionStore.subscribe((state) => { const status = state.status + + // When going offline, clear typing states to prevent stale indicators + // and orphaned typing timeout timers + if (status !== 'online' && previousStatus === 'online') { + if (debug) console.log('[SideEffects] Chat: Going offline, clearing typing states') + chatStore.getState().clearAllTyping() + } + // When we come back online after being disconnected if (status === 'online' && previousStatus !== 'online') { const activeConversationId = chatStore.getState().activeConversationId From 858fb081411d8d44371c67aeb519d7b3e2176a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20R=C3=A9mond?= Date: Fri, 13 Feb 2026 09:08:00 +0100 Subject: [PATCH 2/3] fix(sdk): wrap XState subscription for cleanup type compatibility presenceActor.subscribe() returns a Subscription object, not a function. Wrap it in an arrow function to match cleanupFunctions type signature. --- packages/fluux-sdk/src/core/XMPPClient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fluux-sdk/src/core/XMPPClient.ts b/packages/fluux-sdk/src/core/XMPPClient.ts index 77113a1..4c6920d 100644 --- a/packages/fluux-sdk/src/core/XMPPClient.ts +++ b/packages/fluux-sdk/src/core/XMPPClient.ts @@ -340,10 +340,10 @@ export class XMPPClient { }).start() // Subscribe to persist state changes to sessionStorage - const unsubscribePresencePersist = this.presenceActor.subscribe(() => { + const presencePersistSubscription = this.presenceActor.subscribe(() => { savePresenceSnapshot(this.presenceActor) }) - this.cleanupFunctions.push(unsubscribePresencePersist) + this.cleanupFunctions.push(() => presencePersistSubscription.unsubscribe()) // Create presence options that read from the machine (single source of truth) const presenceOptions: DefaultStoreBindingsOptions = { From 3e0ca1c847e2f8c6f5af9341b8553b9a55a73cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20R=C3=A9mond?= Date: Fri, 13 Feb 2026 09:16:50 +0100 Subject: [PATCH 3/3] fix(sdk): support React StrictMode with proper binding lifecycle - Fix TS2345: wrap XState Subscription in arrow function for cleanupFunctions type compatibility - Extract setupBindings() from constructor so bindings can be re-established after destroy() - Split cleanup into bindingCleanupFunctions (rebindable: store bindings, side effects, presence sync) and cleanupFunctions (constructor-only: presence persistence) - XMPPProvider now calls setupBindings/destroy in useEffect, supporting StrictMode's mount/cleanup/remount cycle --- packages/fluux-sdk/src/core/XMPPClient.ts | 49 +++++++++++++------ .../fluux-sdk/src/provider/XMPPProvider.tsx | 12 ++++- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/packages/fluux-sdk/src/core/XMPPClient.ts b/packages/fluux-sdk/src/core/XMPPClient.ts index 4c6920d..a2f0bde 100644 --- a/packages/fluux-sdk/src/core/XMPPClient.ts +++ b/packages/fluux-sdk/src/core/XMPPClient.ts @@ -277,8 +277,8 @@ export class XMPPClient { private modulesInitialized = false /** - * Cleanup functions for store subscriptions. - * Called in destroy() to prevent memory leaks. + * Cleanup functions for all subscriptions and bindings. + * Torn down in destroy() and re-established by setupBindings(). * @internal */ private cleanupFunctions: (() => void)[] = [] @@ -339,11 +339,8 @@ export class XMPPClient { snapshot: persistedSnapshot, }).start() - // Subscribe to persist state changes to sessionStorage - const presencePersistSubscription = this.presenceActor.subscribe(() => { - savePresenceSnapshot(this.presenceActor) - }) - this.cleanupFunctions.push(() => presencePersistSubscription.unsubscribe()) + // Note: presence persistence subscription is set up in setupBindings() + // so it can be re-established after destroy() in React StrictMode. // Create presence options that read from the machine (single source of truth) const presenceOptions: DefaultStoreBindingsOptions = { @@ -391,6 +388,34 @@ export class XMPPClient { // Initialize with default store bindings (using global Zustand stores) this.initializeModules(createDefaultStoreBindings(presenceOptions)) + // Set up all bindings (presence sync, store bindings, side effects). + // Extracted to a method so XMPPProvider can call setupBindings/destroy + // in useEffect for proper React StrictMode support. + this.setupBindings() + } + + /** + * Set up store bindings, presence sync, and side effects. + * + * This wires SDK events to Zustand store updates, sets up presence + * synchronization, and initializes store-based side effects. + * + * Can be called after {@link destroy} to re-establish bindings + * (used by XMPPProvider for React StrictMode compatibility). + */ + setupBindings(): void { + // Clean up any existing bindings first (idempotent) + for (const cleanup of this.cleanupFunctions) { + try { cleanup() } catch { /* ignore */ } + } + this.cleanupFunctions = [] + + // Subscribe to persist presence state changes to sessionStorage + const presencePersistSubscription = this.presenceActor.subscribe(() => { + savePresenceSnapshot(this.presenceActor) + }) + this.cleanupFunctions.push(() => presencePersistSubscription.unsubscribe()) + // Set up presence sync (machine state -> XMPP presence) const unsubscribePresenceSync = this.setupPresenceSync(this.presenceActor) this.cleanupFunctions.push(unsubscribePresenceSync) @@ -886,14 +911,10 @@ export class XMPPClient { * ``` */ destroy(): void { - // Clean up all store subscriptions and side effects to prevent memory leaks. - // This is called when XMPPProvider unmounts or when the client is destroyed. + // Clean up all subscriptions (store bindings, side effects, presence sync, + // presence persistence) to prevent memory leaks for (const cleanup of this.cleanupFunctions) { - try { - cleanup() - } catch { - // Ignore cleanup errors - } + try { cleanup() } catch { /* ignore */ } } this.cleanupFunctions = [] diff --git a/packages/fluux-sdk/src/provider/XMPPProvider.tsx b/packages/fluux-sdk/src/provider/XMPPProvider.tsx index f4288ce..5bbdea3 100644 --- a/packages/fluux-sdk/src/provider/XMPPProvider.tsx +++ b/packages/fluux-sdk/src/provider/XMPPProvider.tsx @@ -135,10 +135,18 @@ export function XMPPProvider({ clientRef.current = new XMPPClient(config) } - // Clean up client on unmount (handles store bindings + presence cleanup) + // Manage store bindings lifecycle in useEffect for React StrictMode support. + // StrictMode runs effects, then cleanup, then effects again. By setting up + // bindings here (not just in the constructor), the cycle works correctly: + // setup bindings → cleanup (destroy bindings) → setup bindings again. + // The constructor also calls setupBindings() for non-React usage. useEffect(() => { + const client = clientRef.current + if (!client) return + // Re-establish bindings (idempotent: destroy() clears previous ones first) + client.setupBindings() return () => { - clientRef.current?.destroy() + client.destroy() } }, [])