fix(net): a node-interest burst overflows the outbound queue and silently disconnects the player #291

Open
opened 2026-08-09 17:52:59 +02:00 by panda · 0 comments
Collaborator

A long /tp drops the connection. Reproduced 3x live, then instrumented to root cause (instrumentation reverted; the tree is clean).

Symptom

Client: Connection lost: The remote party closed the WebSocket connection without completing the close handshake.
Server: a plain "disconnected", no error, no metric. The teleport commits — you log back in at the destination.

Root cause

Instrumented, teleporting to (2192,2560) (northern_grassland):

[DIAG] node diff observer=... entered=349 left=0

349 NaturalNodeSpawned in ONE tick (WorldTick sends one message per node). WebSocketConnection queues into Channel.CreateBounded(OutboundCapacity) with OutboundCapacity = 256 and treats a full queue as "client hopelessly behind" -> Close(), which cancels the receive loop and kills the socket with no close frame.

The 349 is structural, not incidental: EcologyScatter yields one candidate per spacing x spacing slot, so temperate_pine spacing 6 = 1/36 = 2.78%; the node view block is SpawnerChunkSize 16 x NodeViewChunkRadius 3 = 7x7 chunks = 112x112 = 12,544 cells. 12,544/36 = 348.4.

It is intermittent because the send pump drains concurrently: it survives when the client is idle (a login in the same block survived 3/3) and fails when the client is busy rebuilding terrain mesh — i.e. exactly during a teleport. Walking is safe: the block shifts by one chunk, not a whole block.

Why it matters beyond /tp

The same reconcile runs on connect (InterestManager.Enter starts the known set empty), so the burst is a coin flip on login too. Any of these makes it universal rather than intermittent: filling the empty biomes with species rows, moving the spawn into grass/northern_grassland, raising NodeViewChunkRadius (which its own comment invites for 4K), or a second player in the same block.

Scope

  • Batch the diff: NaturalNodesSpawned(IReadOnlyList<NaturalNodeState>) / NaturalNodesRemoved(IReadOnlyList<Guid>), one guarded send each, mirrored in the client dispatcher. Bump ProtocolVersion.Current (21 today) in the SAME change.
  • Make the overflow visible: a Warning naming the player and the cause, and an isommo_outbound_overflow_total counter next to the malformed-message one.
  • Move the capacity onto a bound options record per the typed-options invariant, as headroom — never as the mechanism.
  • Note in EcologyOptions.NodeViewChunkRadius's comment that a block is now one message, so raising it is safe.

Do NOT switch the channel to Wait mode — that blocks the sim thread, which is the exact thing the class exists to prevent.

Definition of Done

  • A WorldTick burst test: an observer standing in a saturated node block produces at most one NaturalNodesSpawned, and the whole reconcile emits fewer messages than the queue capacity.
  • Teleporting into (2192,2560) ten times in a row keeps the connection every time.
A long `/tp` drops the connection. Reproduced 3x live, then instrumented to root cause (instrumentation reverted; the tree is clean). ## Symptom Client: `Connection lost: The remote party closed the WebSocket connection without completing the close handshake.` Server: a plain "disconnected", **no error, no metric**. The teleport commits — you log back in at the destination. ## Root cause Instrumented, teleporting to (2192,2560) (`northern_grassland`): ``` [DIAG] node diff observer=... entered=349 left=0 ``` 349 `NaturalNodeSpawned` in ONE tick (`WorldTick` sends one message per node). `WebSocketConnection` queues into `Channel.CreateBounded(OutboundCapacity)` with **`OutboundCapacity = 256`** and treats a full queue as "client hopelessly behind" -> `Close()`, which cancels the receive loop and kills the socket with no close frame. The 349 is structural, not incidental: `EcologyScatter` yields one candidate per `spacing x spacing` slot, so `temperate_pine spacing 6` = 1/36 = 2.78%; the node view block is `SpawnerChunkSize 16` x `NodeViewChunkRadius 3` = 7x7 chunks = 112x112 = 12,544 cells. 12,544/36 = 348.4. It is intermittent because the send pump drains concurrently: it survives when the client is idle (a login in the same block survived 3/3) and fails when the client is busy rebuilding terrain mesh — i.e. exactly during a teleport. Walking is safe: the block shifts by one chunk, not a whole block. ## Why it matters beyond `/tp` The same reconcile runs on **connect** (`InterestManager.Enter` starts the known set empty), so the burst is a coin flip on login too. Any of these makes it universal rather than intermittent: filling the empty biomes with species rows, moving the spawn into `grass`/`northern_grassland`, raising `NodeViewChunkRadius` (which its own comment invites for 4K), or a second player in the same block. ## Scope - [ ] Batch the diff: `NaturalNodesSpawned(IReadOnlyList<NaturalNodeState>)` / `NaturalNodesRemoved(IReadOnlyList<Guid>)`, one guarded send each, mirrored in the client dispatcher. Bump `ProtocolVersion.Current` (21 today) in the SAME change. - [ ] Make the overflow visible: a Warning naming the player and the cause, and an `isommo_outbound_overflow_total` counter next to the malformed-message one. - [ ] Move the capacity onto a bound options record per the typed-options invariant, as headroom — never as the mechanism. - [ ] Note in `EcologyOptions.NodeViewChunkRadius`'s comment that a block is now one message, so raising it is safe. Do NOT switch the channel to `Wait` mode — that blocks the sim thread, which is the exact thing the class exists to prevent. ## Definition of Done - A `WorldTick` burst test: an observer standing in a saturated node block produces at most one `NaturalNodesSpawned`, and the whole reconcile emits fewer messages than the queue capacity. - Teleporting into (2192,2560) ten times in a row keeps the connection every time.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
marco/IsoMmo#291
No description provided.