felladrin / felladrin/LinkedTimer

handlePeriodicSyncEvent has no older-peer guard, so any peer can start any other

Open
#1,203 0 comments 0 reactions 0 assignees View on GitHub
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.