Throttle persona:state:snapshot — fix 8GB browser memory leak - #288
Conversation
…ser leak emitSnapshot() fired on every recordActivity(), rest(), and updateInboxLoad() with no throttling. 15 personas × multiple calls per cycle = 200+ events/s flooding the WebSocket to browser. EventRateLimiter was blocking at 200 but damage was done. Now throttled to max 1 emit per 2s per persona with trailing edge to ensure latest state always gets sent.
There was a problem hiding this comment.
Pull request overview
This PR fixes an 8GB browser memory leak caused by unthrottled persona:state:snapshot event emissions flooding the WebSocket. The fix adds a leading+trailing throttle (2s per persona) so the latest state always arrives without overwhelming the browser.
Changes:
- Added a 2-second throttle with trailing edge to
emitSnapshot()inPersonaStateManager, splitting the emission logic intoemitSnapshot()(throttle gate) andemitSnapshotNow()(actual emit). - Reused the stored
_lastSnapshotTimeas the payload timestamp instead of callingDate.now()a second time.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery. | ||
| * Without the context, bare Events.emit() stays server-local. | ||
| */ | ||
| /** | ||
| * Throttled snapshot emission — max once per SNAPSHOT_THROTTLE_MS. | ||
| * With 15 personas each calling this on every cycle (3-5s) plus rest(), | ||
| * unthrottled emission hit 200+/s and flooded the WebSocket to browser. | ||
| */ |
There was a problem hiding this comment.
Two consecutive JSDoc blocks are stacked on emitSnapshot() — the original description (lines 271-275) and the new throttle rationale (lines 276-280). Only the last JSDoc block before a declaration is used by tooling (IDE tooltips, generated docs). The old block is now dead documentation. These should be merged into a single JSDoc comment that covers both the purpose (browser-side snapshot emission via jtagContext) and the throttling behavior.
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery. | |
| * Without the context, bare Events.emit() stays server-local. | |
| */ | |
| /** | |
| * Throttled snapshot emission — max once per SNAPSHOT_THROTTLE_MS. | |
| * With 15 personas each calling this on every cycle (3-5s) plus rest(), | |
| * unthrottled emission hit 200+/s and flooded the WebSocket to browser. | |
| */ | |
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery; | |
| * without the context, bare Events.emit() stays server-local. | |
| * | |
| * Emission is throttled — max once per SNAPSHOT_THROTTLE_MS. With many personas | |
| * each calling this on every cycle (3–5s) plus rest(), unthrottled emission | |
| * can reach 200+/s and flood the WebSocket to the browser. | |
| */ |
| /** | ||
| * PersonaStateManager: Manages internal state and traffic decisions | ||
| */ | ||
| /** Minimum interval between snapshot emissions per persona (ms) */ | ||
| const SNAPSHOT_THROTTLE_MS = 2000; | ||
|
|
There was a problem hiding this comment.
The SNAPSHOT_THROTTLE_MS constant was inserted between the class-level JSDoc (lines 49-51: PersonaStateManager: Manages internal state and traffic decisions) and the class PersonaStateManager declaration at line 55. In TypeScript/JSDoc, a /** */ comment documents the next declaration — so the class-level JSDoc now documents SNAPSHOT_THROTTLE_MS instead of the class, and the class itself loses its documentation. Move the constant above the class JSDoc or inside the class as a private static readonly to preserve the original documentation association.
| /** | |
| * PersonaStateManager: Manages internal state and traffic decisions | |
| */ | |
| /** Minimum interval between snapshot emissions per persona (ms) */ | |
| const SNAPSHOT_THROTTLE_MS = 2000; | |
| /** Minimum interval between snapshot emissions per persona (ms) */ | |
| const SNAPSHOT_THROTTLE_MS = 2000; | |
| /** | |
| * PersonaStateManager: Manages internal state and traffic decisions | |
| */ |
| const now = Date.now(); | ||
| if (now - this._lastSnapshotTime < SNAPSHOT_THROTTLE_MS) { | ||
| // Schedule a trailing emit so the latest state always gets sent | ||
| if (!this._snapshotPending) { | ||
| this._snapshotPending = true; | ||
| setTimeout(() => { | ||
| this._snapshotPending = false; | ||
| this.emitSnapshotNow(); | ||
| }, SNAPSHOT_THROTTLE_MS - (now - this._lastSnapshotTime)); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| this.emitSnapshotNow(); |
There was a problem hiding this comment.
There's a subtle edge case: when a leading-edge emit passes the throttle check while _snapshotPending is still true from a previously scheduled trailing timer, both the leading emit and the trailing timer will fire, causing a double-emit within milliseconds. This happens when setTimeout fires slightly late and a new call arrives just after the throttle window expires but before the pending timer runs.
To fix, either (a) store the timer ID and clear it when a leading emit fires, or (b) reset _snapshotPending = false at line 297 before calling this.emitSnapshotNow() in the leading path. Option (a) is more robust:
Store the timeout handle (e.g., private _snapshotTimer: ReturnType<typeof setTimeout> | null = null) and call clearTimeout(this._snapshotTimer) before the leading-edge emitSnapshotNow() call.
Summary
emitSnapshot()in PersonaState was firing on everyrecordActivity(),rest(), andupdateInboxLoad()with zero throttlingTest plan
npm run build:ts— zero errorsnpm start— system comes upEVENT TRENDING/EVENT CASCADE BLOCKEDwarnings (was immediate before)