Skip to content

fix: cap session event queue and clear stale resume timers - #241

Closed
snakkeeh wants to merge 1 commit into
PerformanC:devfrom
snakkeeh:dev
Closed

snakkeeh wants to merge 1 commit into
PerformanC:devfrom
snakkeeh:dev

Conversation

@snakkeeh

@snakkeeh snakkeeh commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Changes

  • Added a central queueSessionEvent helper (in utils.ts) that buffers player events for paused, resumable sessions with a hard cap of 500 events using drop-oldest semantics, so the newest state survives for replay on resume. Warns at 50% capacity and when the cap is reached.
  • Added a clearSessionEventQueue helper that drains the queue and logs the discarded count; used on session destroy and after the resume flush.
  • Wired both event-queue write sites (player emitEvent and the main-process playerEvent IPC dispatcher) through the helper, removing duplicated raw push logic.
  • The player-side gate now also respects the session's resuming flag (passed through playerManager via the playback Session type), so events are dropped instead of buffered when resuming is disabled, consistent with the dispatcher behavior.
  • SessionManager.pause() now clears any stale timeoutFuture before scheduling a new destroy timer, preventing leaked handles and premature session destruction on pause/resume cycles.

Why

  • The event queue had no size limit: a client that disconnects while its players keep playing would grow the queue unboundedly (every TrackStart/End/progress event), causing unbounded memory growth on long-running nodes.
  • Scheduling a new resume timer without clearing the previous one could leak timers and destroy a re-paused session earlier than configured.

Checkmarks

  • I have tested the changes locally (capped-queue behavior and resume flush verified programmatically).
  • I have run the linter/formatter (code follows the existing style conventions).
  • I have tested the changes on a real bot (manual session pause/resume flow).

Additional info

The cap value (500) is intentionally conservative; each buffered event is a serialized JSON string, so worst-case memory per paused session stays in the low hundreds of KB.

@1Lucas1apk

Copy link
Copy Markdown
Member

I'm closing this for now because the player refactor in #250 is still in progress, and the event queue changes will need to be integrated into the new PlayerEvents implementation as well to ensure the queue limit is preserved.

There's also a logging concern: once the queue reaches its 500-event limit, every subsequent event can trigger another warning, which could create unnecessary log spam while a session remains disconnected.

The queue limit itself makes sense, but I'd prefer to revisit this once the refactor is settled and the queue handling and warning behavior can be addressed consistently.

@1Lucas1apk 1Lucas1apk closed this Oct 21, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants