felladrin / felladrin/LinkedTimer
handlePeriodicSyncEvent has no older-peer guard, so any peer can start any other
- Dominant language
- TypeScript
- Stars
- 6
- Forks
- 2
- Avg merge
- 3h 51m
- Merged PRs (30d)
- 33
Description
`handleInitialSyncEvent` ignores events from peers that joined later:
```ts
const isReceivingThisEventFromAPeerThatJoinedLater = joinRoomTimestamp > (getRoom()?.creationTimestamp ?? 0);
if (isReceivingThisEventFromAPeerThatJoinedLater) return;
```
`handlePeriodicSyncEvent` has no such guard, because `PeriodicSyncParameters` carries no timestamp. Any peer can therefore pull any other peer into running, in either direction:
```ts
if (isRunning && Math.abs(totalSeconds - getTotalTimerSeconds()) > 1) {
startTimerWithValues(timeValues);
}
```
That is what made the bug in #1198 stick rather than settle: a running client joining a stopped room ignored the room's state, kept ticking, and its own periodic syncs then started everyone else. #1198 closes the entry point, but the mechanism is still there for any future path that leaves one peer running in a stopped room.
Closing it means adding `joinRoomTimestamp` to `PeriodicSyncParameters` and applying the same guard. That is a wire-format change, so peers on different versions have to keep working through it: an older peer sends no timestamp, and the handler needs a decision for that case.
It also closes the damage path in #1202, since the frame described there would be ignored by any peer that joined earlier.
For the test side, `tests/initial-sync.test.mjs` stubs `listenToPeriodicSync() {}` as a no-op, so covering this needs that stub to capture the handler the way `listenToInitialSync` already does.
Found while reviewing #1198.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.