cloudflare / cloudflare/partykit

withYjs: one peer disconnecting wipes every peer's awareness for ~15s

Open
#421 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
1.3k
Forks
79
PR merge metrics
No merged PRs in 30d

Description

## What happens

In `withYjs`, the connection→clientID ownership map is built from the `added` array of awareness `update` events. `added` means "the document hasn't seen this clientID before", which is a property of the document, not of the connection that sent the update.

Yjs clients relay other peers' awareness states without preserving the origin. So when peer B's socket forwards peer A's state, A's clientID is `added` under B's connection, and B gets recorded as A's owner. `removeAwarenessStates` deletes from `states` but keeps `meta`, so a clientID is only ever `added` once. The misattribution is permanent for the object's lifetime.

On `onClose`, that connection's whole ownership list goes to `removeAwarenessStates`. So closing one connection removes the awareness state of every peer whose clientID it happened to relay first. The remaining peers can't see each other until Yjs re-announces local state, about 15 seconds later.

## Where

`packages/y-partyserver/src/server/index.ts` on current `main`:

L285-292, the ownership map is credited with `added`:
```ts
if (conn !== null) {
const currentIds = new Set(getAwarenessIds(conn));
for (const clientID of added) currentIds.add(clientID);
for (const clientID of removed) currentIds.delete(clientID);
setAwarenessIds(conn, [...currentIds]);
}
```

L497-509, `onClose` removes it:
```ts
const controlledIds = getAwarenessIds(connection);
if (controlledIds.length > 0)
awarenessProtocol.removeAwarenessStates(this.document.awareness, controlledIds, null);
```

## Reproduction

Three peers in one room, each sending only its own awareness state. Because clients relay, one connection ends up credited with the others' clientIDs. Close that connection and every peer's awareness is removed.

Reproduced in workerd (`@cloudflare/vitest-pool-workers`), both `hibernate: true` and `hibernate: false`. Present since #341 (2.1.0), still on `main` and in published `2.2.0`.

## Fix

Make ownership last-writer-wins and exclusive. Claim on `added` and `updated` so a peer's own refresh reclaims its clientID, and revoke the claimed ids from every other connection's list:

```ts
const claimed = new Set([...added, ...updated]);
for (const other of this.getConnections()) {
if (other === conn) continue;
const otherIds = getAwarenessIds(other);
const kept = otherIds.filter((id) => !claimed.has(id));
if (kept.length !== otherIds.length) setAwarenessIds(other, kept);
}
for (const clientID of claimed) currentIds.add(clientID);
```

A relayed clientID stays misattributed until its owner's next frame, so the window shrinks to the Yjs refresh interval rather than closing completely. Fully closing it would need an ownership signal in the protocol itself.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.