Skip to content

refactor: one Deadline primitive behind every bounded wait in the connection lifecycle - #263

Closed
diegolmello wants to merge 5 commits into
mobilefrom
arch/01
Closed

refactor: one Deadline primitive behind every bounded wait in the connection lifecycle#263
diegolmello wants to merge 5 commits into
mobilefrom
arch/01

Conversation

@diegolmello

@diegolmello diegolmello commented Aug 12, 2026

Copy link
Copy Markdown
Member

Proposed changes

The Socket holds four waits it must be able to give up on: reopenNow waits for open, probe waits for pong, waitForOpen waits for open, and the Liveness chain waits for the answer to its ping. Each one wrote out the same sequence by hand — register a once, arm a timer, guard a settled boolean, then detach the listener and clear the timer on whichever arrived first.

That sequence is load-bearing rather than incidental: ADR-0002 names these exact four waits as the reason the SDK emitter had to replace off and emit. It was written four times, so a fifth wait meant getting it right a fifth time.

This adds one private bounded wait on the Socket and rebuilds all four on it. It listens for a named DDP event, gives up on the Deadline, and detaches the listener and clears the timer on either outcome. Its start callback runs with the listener already attached, so a server answering in the same tick as the write cannot be missed, and returning false from it abandons the wait cleanly — that is how a Probe reports a transport that would not take the ping.

The Liveness chain becomes a repeated Probe. A Probe and one turn of the chain are the same act — write a ping, wait for the pong, give up on a Deadline — but they were two separate implementations that had already drifted, each needing its own fix for the same class of fault. The chain is now: probe on the interval, schedule the next turn when the server answers, Reopen when it does not.

Judgement calls, and the behaviour that changes with them:

  • The Deadline stays on the Probe rather than moving into send, so no other caller inherits a reply timeout. ADR-0003 decided that, and it still holds — only the mechanism moved, so this is recorded as an amendment to it rather than a quiet override.
  • The wait answers one question, as true or false: did the event arrive in time. A refused write short circuits to that same false answer rather than reporting itself separately, because no caller distinguishes the two — both mean the Socket cannot be trusted, and both lead to a Reopen. Only a refusal is a signal, so a start with nothing to report returns nothing.
  • The chain's ping is now written straight to the transport rather than through send. The frame on the wire is identical — send assigns no id to a ping — but the chain no longer consumes an id from the sent counter and no longer registers the disconnected listener send attaches. Nothing reads that counter for anything but uniqueness.
  • A chain turn reopens at once whenever the transport will not take the ping: because the write threw, or because the socket is no longer open. The second case previously went through send into waitForOpen and waited up to twice the Reopen interval before failing into the Reopen. A socket that cannot be written to is dead now, not two intervals from now.
  • The mirror of that, and the one behaviour this loses: a connection that drops after the ping was written. The turn holds a Deadline and a pong listener, and nothing watching for the drop, so it now waits the Deadline out before reopening where send's disconnected listener would have failed it immediately. Bounded by one ping interval, and the turn that follows finds the socket unwritable and fast-fails.
  • The reopen promise is now cleared a microtask after the open arrives rather than synchronously inside the emit, so a caller asking for an immediate reconnect from inside an open listener joins the reopen that is settling instead of starting a new one. That was already true of any listener registered ahead of the old cleanup; it is now true of all of them, and it is pinned by a test.
  • The settled booleans are gone rather than centralised. Settling removes the listener and clears the timer together, so neither racer reaches the promise twice; where a wait is abandoned in the same tick as its event arrives, the second off is a miss, harmless only because ADR-0002 made a missed off a no-op. The primitive now depends on that guarantee, and the ADR says so.

SDKEventEmitter gains listenerCount, which the emitter had no way to answer: the only read it offered also destroyed what it read, so a test could ask once per object and mutated the thing it was inspecting. The tests that check a wait left no listener behind now ask without changing anything. ISocket is unchanged — counting listeners is an observation about internal wiring, not part of driving a socket.

Recorded as ADR-0005. The Liveness chain and Probe entries in the domain language were sharpened to say the chain is one Probe per interval; no new term was needed, since Deadline, Probe, Liveness chain and Reopen were all already named. The deadline parameters of both probe and waitForOpen were renamed off "timeout", which the domain language reserves for the config option.

Steps to reproduce

  • Connect a client to a server and leave it idle past two ping intervals. The chain keeps probing and the connection stays up.
  • Silence the server's pongs without closing the socket. One interval later the Probe's Deadline expires, the socket reads as disconnected, and a Reopen is scheduled and then builds a replacement socket.
  • Have the transport throw from a write, or close under the client, while the chain is running. The Reopen is scheduled on that turn rather than an interval later.
  • Drop the connection immediately after a ping goes out. The Reopen is scheduled when the Deadline expires, one interval later.
  • Call an immediate reconnect twice concurrently. One replacement socket is built, both callers share one promise, and in-flight sends are rejected once.
  • Ask for an immediate reconnect from inside an open listener. It joins the reconnect that is settling instead of building another socket.

Tests

  • probe abandons its deadline when the transport refuses the ping write — leaves no pending timer and no pong listener behind.
  • The ping chain leaves no pong listener behind however many turns it runs, so a long-lived socket cannot pile up one listener per turn.
  • The ping chain reopens without waiting out the deadline when the transport refuses the ping write.
  • The ping chain reopens without waiting out the deadline when the socket is no longer open.
  • The ping chain waits out the deadline when the connection drops after the ping is written — the disclosed loss above, pinned so it cannot change unnoticed. Fails if the chain is routed back through send.
  • An immediate reconnect is still joinable from an open listener that runs after the wait settles, asserted by promise identity, since two distinct promises serialise the same and only identity catches a second reconnect starting.
  • listenerCount counts an unfired once, returns 0 for an event never registered, and can be asked twice before following a real removal down to 0.
  • All 117 tests that existed before this branch are unchanged and still pass; none needed updating for this refactor.

The wait now answers one question — did the event arrive in time — and only a
refused write is a signal, so a `start` with nothing to report returns nothing.
`reopenNow` holds the same promise it returns by construction rather than by
statement order, and the Deadline timer is declared before the code that clears
it.

Two consequences the ADR stated were unenforced: a chain turn waiting out its
Deadline when the connection drops after the ping is written, and a reconnect
staying joinable from an `open` listener. Both are pinned, and both fail on the
change that would undo them.

`SDKEventEmitter` gains `listenerCount`, so the tests that check a wait left no
listener behind stop counting by deleting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant