daemon: a FIN waits for the data sent before it

[TeoSlayer]

Problem

A FIN that overtook a lost segment was acted on at once:

• the receiver closed the stream with the segment missing, so the reader saw EOF without it;
• its FIN-ACK acknowledged the FIN's sequence number, so the sender cleared its unacknowledged data, including the segment it was about to resend.

Every version does this. It is the cause of the pub/sub losses in the v1.17.0 lab run: at 5% loss, 4 of 30 events never reached the broker while pilotctl publish reported success, and the broker logged unexpected EOF. Any client that closes right after writing is exposed the same way.

Fix

Receiver.

• An established connection that gets a FIN ahead of missing data records it (finPending/finSeq, the first copy's sequence number) and sends an ACK asking for the gap.
• It handles the FIN (acceptFIN: close the receive side, TIME_WAIT, FIN-ACK) once in-order delivery reaches the FIN's sequence number.
• If nothing more comes from the peer for 15 s (finHoldIdle), it handles the FIN anyway (expireHeldFIN), and the reader gets EOF without the missing data, as before. A sender still there resends at most 10 s apart (RTOMax), so it always stays inside the bound. Without the bound, a sender that stopped with a segment lost (any version, stopped or restarted) left the stream open until keepalive gave up, about two minutes later.
• A connection this side has already closed handles a FIN at once, as before, since it discards the peer's data anyway.

Sender.

• CloseConnection reserves the FIN's sequence number as before.
• When data is still unacknowledged, it holds the FIN on the connection. releaseHeldFIN sends it in whichever of these comes first:
  • the ACK that acknowledges the last data (the ACK path);
  • the end of the 5 s linger (finLinger, a timer);
  • the reaper removing the connection;
  • shutdown.
• No goroutine waits, and the caller is not blocked. A held FIN is dropped only if the connection has already closed (TIME_WAIT/CLOSED), where the FIN-ACK this side sent stands for it.
• This protects the end of a write on receivers that have not upgraded.

Shutdown. Stopping the daemon sends a FIN that is still held, and no second FIN after it. CloseConnection reserves the FIN's sequence number and holds the FIN in one critical section, so shutdown never sees one without the other.

A restarted peer. A SYN on the ports of a connection whose peer FIN is held comes from a restarted peer dialling again, whose ephemeral ports start over. It is not a resend: a peer that sent a FIN is past its SYN.

• It ends the old connection quietly, because a FIN-ACK would reach the new one.
• It opens a new connection. Answered as the old connection, the new one's data was acknowledged as duplicates and never delivered.

The hold ends with the connection. An RST, a local close, the reaper or shutdown each stop the hold timer, and the timer acts only on an established connection, so it never sends a FIN-ACK for a connection that is gone.

Interop:

• A new receiver with an old sender: the receiver rule protects the data.
• An old receiver with a new sender: the FIN arrives after the data is acknowledged.
• Wire format and flags are unchanged.

Tests

zz_fin_in_order_test.go:

• TestEarlyFINIsHeldUntilTheDataBeforeItArrives: the reply to the early FIN is a plain ACK asking for the gap, the state stays established, and the reader gets every byte before EOF.
• TestFINOnAClosingConnectionIsAcceptedAtOnce.
• TestCloseSendsTheFINAfterTheLingerIfNothingIsAcknowledged.
• TestHeldFINGoesOutWithTheACKOfTheLastData.
• TestHeldPeerFINIsHandledOnceThePeerFallsSilent, where arriving data restarts the wait.
• TestStopSendsAFINThatWasWaitingForData and TestStopSendsOnlyTheHeldFINOfAHalfDoneClose.
• TestSYNOnAConnectionWithAHeldFINIsANewConnection.
• TestHoldEndsWithTheConnection.
• TestWriteThenCloseSurvivesALostSegment: two daemons, one segment's first send dropped, with the FIN sent at once (old senders) and with the FIN held.

Each fails with its fix reverted.

Results:

• TestWriteThenCloseSurvivesALostSegment under -race -count=300: 0 failures. A review of the first version found 3 in 300, a race that dropped the held FIN, now fixed.
• go test -race ./pkg/... ./cmd/...: passes.
• ./tests: passes.

Existing tests adjusted because the FIN now waits for acknowledged data:

• TestHandleStreamFINEstablishedClosesAndSendsFINACK sets ExpectedSeq to the FIN's sequence number.
• TestCloseSendsHeldTailBeforeFIN acknowledges the data through the ACK path's release.
• TestCloseCommitsTheWholeTailDespiteSendErrors checks the held FIN's sequence number.

Not changed

• A FIN that goes out after its data is acknowledged is not resent if lost. The retransmit loop leaves FIN_WAIT once no data is outstanding, as it does in every version; the peer's keepalive then closes its side.
• If a writer is blocked on the window when another goroutine closes the connection, the writer can still send data numbered after the FIN once the window opens. That predates this PR (found reviewing daemon: a send error no longer strands the end of a write; no RTT sample from a resent segment's ACK #518); the receiver drops data past the FIN.

🤖 Generated with Claude Code