refactor: one Deadline primitive behind every bounded wait in the connection lifecycle - #263
Closed
diegolmello wants to merge 5 commits into
Closed
refactor: one Deadline primitive behind every bounded wait in the connection lifecycle#263diegolmello wants to merge 5 commits into
diegolmello wants to merge 5 commits into
Conversation
…nection lifecycle
…oned-wait paths, drop restated comments
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
The Socket holds four waits it must be able to give up on:
reopenNowwaits foropen,probewaits forpong,waitForOpenwaits foropen, and the Liveness chain waits for the answer to its ping. Each one wrote out the same sequence by hand — register aonce, arm a timer, guard asettledboolean, 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
offandemit. 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
startcallback 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:
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.startwith nothing to report returns nothing.send. The frame on the wire is identical —sendassigns no id to a ping — but the chain no longer consumes an id from thesentcounter and no longer registers thedisconnectedlistenersendattaches. Nothing reads that counter for anything but uniqueness.sendintowaitForOpenand 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.send'sdisconnectedlistener would have failed it immediately. Bounded by one ping interval, and the turn that follows finds the socket unwritable and fast-fails.openarrives rather than synchronously inside the emit, so a caller asking for an immediate reconnect from inside anopenlistener 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.settledbooleans 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 secondoffis a miss, harmless only because ADR-0002 made a missedoffa no-op. The primitive now depends on that guarantee, and the ADR says so.SDKEventEmittergainslistenerCount, 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.ISocketis 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
probeandwaitForOpenwere renamed off "timeout", which the domain language reserves for the config option.Steps to reproduce
openlistener. It joins the reconnect that is settling instead of building another socket.Tests
probeabandons its deadline when the transport refuses the ping write — leaves no pending timer and noponglistener behind.ponglistener behind however many turns it runs, so a long-lived socket cannot pile up one listener per turn.send.openlistener 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.listenerCountcounts an unfiredonce, returns 0 for an event never registered, and can be asked twice before following a real removal down to 0.