Summary
ShapeStream's lifecycle is tracked through independent boolean flags (#started, #connected) plus implicit state from #pauseLock.isPaused and #subscribers.size. Because these are independent, invalid state combinations are representable and can cause resource leaks.
Example: unsubscribeAll doesn't prevent restart
After unsubscribeAll():
- Subscribers are cleared, timers are torn down
- But
#started remains true
- If a pending pause lock releases,
onReleased checks #started → true → calls #start()
#start() re-arms wake detection timers and makes HTTP requests with zero subscribers
Related: empty .catch(() => {}) blocks
Two places swallow all errors from #start():
onReleased callback (line ~642): .catch(() => {}) with a comment claiming errors are "handled internally via onError" — but only if the user configured an onError handler. Without one, the stream silently freezes.
requestSnapshot (line ~1744): same pattern.
These are pre-existing but become more impactful as more code paths flow through #start().
Potential approaches
Minimal fix: Add this.#started = false to unsubscribeAll() to prevent ghost restarts.
Deeper fix: Collapse the boolean flags into a state enum (idle | running | paused | stopped) so invalid combinations are unrepresentable. This would also make the empty .catch blocks easier to reason about — a stopped state would prevent restart attempts rather than relying on flag checks.
Context
Found during review of #3814 (wake detection) fix for pause/resume re-arming. The wake detection change slightly amplifies this issue since timers now re-arm in #start(), but the root cause predates it.
Summary
ShapeStream's lifecycle is tracked through independent boolean flags (#started,#connected) plus implicit state from#pauseLock.isPausedand#subscribers.size. Because these are independent, invalid state combinations are representable and can cause resource leaks.Example:
unsubscribeAlldoesn't prevent restartAfter
unsubscribeAll():#startedremainstrueonReleasedchecks#started→ true → calls#start()#start()re-arms wake detection timers and makes HTTP requests with zero subscribersRelated: empty
.catch(() => {})blocksTwo places swallow all errors from
#start():onReleasedcallback (line ~642):.catch(() => {})with a comment claiming errors are "handled internally via onError" — but only if the user configured anonErrorhandler. Without one, the stream silently freezes.requestSnapshot(line ~1744): same pattern.These are pre-existing but become more impactful as more code paths flow through
#start().Potential approaches
Minimal fix: Add
this.#started = falsetounsubscribeAll()to prevent ghost restarts.Deeper fix: Collapse the boolean flags into a state enum (
idle | running | paused | stopped) so invalid combinations are unrepresentable. This would also make the empty.catchblocks easier to reason about — astoppedstate would prevent restart attempts rather than relying on flag checks.Context
Found during review of #3814 (wake detection) fix for pause/resume re-arming. The wake detection change slightly amplifies this issue since timers now re-arm in
#start(), but the root cause predates it.