-
Notifications
You must be signed in to change notification settings - Fork 311
Description
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
#startedremainstrue - If a pending pause lock releases,
onReleasedchecks#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():
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 = 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.