From 21f477b86e5d4f39370a7736d76d04379c3be933 Mon Sep 17 00:00:00 2001 From: Derek den Haas Date: Tue, 25 Aug 2026 00:36:48 +0200 Subject: [PATCH] fix(tcp): retransmit unacknowledged data after a local close (#182) ControlBlock.Send refused any outgoing segment carrying data in FIN-WAIT-1, citing RFC 9293's "no further SENDs from the user will be accepted by the TCP implementation". That rule bounds what the application may queue, which Handler.Write already enforces, and not the retransmission of data the connection has already accepted from it. The consequence is that write-then-close, which is what nearly every server does with a response, cannot recover from losing its last data segment. The FIN occupies a sequence number above that data, so the peer cannot cross the gap to process the close: it waits for bytes that are never resent while the sender waits for an ACK that cannot arrive. Neither side times out at the TCP layer. FIN-WAIT-2 keeps the restriction and gains the reasoning: it is reached by our FIN being acknowledged, which acknowledges everything below it, so no unacknowledged data can remain there. PendingSegment needs the same distinction, since it decides whether to offer send-buffer data at all; it gets an unexported State predicate rather than a new exported one. Two tests, the second red before the change: - retransmission after RTO expiry, covering the Handler/LossRecovery seam that the RTO unit tests do not reach (they exercise the state machine in isolation, where it behaves correctly). - retransmission after a close with unacknowledged data. Co-authored-by: Derek den Haas --- tcp/control.go | 13 ++-- tcp/definitions.go | 8 +++ tcp/rtointegration_test.go | 120 +++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 tcp/rtointegration_test.go diff --git a/tcp/control.go b/tcp/control.go index 16960a7..31b91da 100644 --- a/tcp/control.go +++ b/tcp/control.go @@ -279,10 +279,9 @@ func (tcb *ControlBlock) PendingSegment(payloadLen int) (_ Segment, ok bool) { // Optimist Strategy: retransmit oldest data once. return Segment{SEQ: tcb.snd.UNA, DATALEN: Size(payloadLen), ACK: tcb.rcv.NXT, WND: tcb.rcv.WND, Flags: FlagACK}, true } - established := tcb._state == StateEstablished - canSendData := established || tcb._state == StateCloseWait + canSendData := tcb._state.txQueuedDataOpen() if !canSendData { - payloadLen = 0 // Can't send data if not established or close-wait. + payloadLen = 0 // No send-buffer data may go out in this state. } if pending == 0 && payloadLen == 0 { return Segment{}, false // No pending segment. @@ -522,8 +521,12 @@ func (tcb *ControlBlock) validateOutgoingSegment(seg Segment) (err error) { err = errSeqNotInWindow } - case seg.DATALEN > 0 && (tcb._state == StateFinWait1 || tcb._state == StateFinWait2): - err = errConnectionClosing // Case 1: No further SENDs from the user will be accepted by the TCP implementation. + case seg.DATALEN > 0 && tcb._state == StateFinWait2: + // FIN-WAIT-2 means our FIN was acknowledged, so no data below it can be + // unacknowledged and data here is a caller error. FIN-WAIT-1 is excluded: + // its FIN sits above data the peer may still be missing, which must go out + // for either side to make progress (RFC 9293 §3.10.8). + err = errConnectionClosing case checkSeq && tcb.snd.WND == 0 && seg.DATALEN > 0 && seg.SEQ == tcb.snd.NXT: err = errZeroWindow diff --git a/tcp/definitions.go b/tcp/definitions.go index eff90e2..df50534 100644 --- a/tcp/definitions.go +++ b/tcp/definitions.go @@ -329,6 +329,14 @@ func (s State) TxDataOpen() bool { return s == StateEstablished || s == StateCloseWait } +// txQueuedDataOpen returns true if already-queued send-buffer data may still be +// put on the wire. It stays true after a local close, where the FIN occupies a +// sequence above data the peer has not acknowledged: until that data is +// (re)transmitted the peer cannot reach the FIN. RFC 9293 §3.10.8. +func (s State) txQueuedDataOpen() bool { + return s.TxDataOpen() || s == StateFinWait1 || s == StateClosing || s == StateLastAck +} + // RxDataOpen returns true if the state allows the receiving of incoming data segments. // Combine with [State.IsPreestablished] to know whether there is no more data to be received over the network. func (s State) RxDataOpen() bool { diff --git a/tcp/rtointegration_test.go b/tcp/rtointegration_test.go new file mode 100644 index 0000000..928fd3d --- /dev/null +++ b/tcp/rtointegration_test.go @@ -0,0 +1,120 @@ +package tcp + +import ( + "math/rand" + "testing" + "time" + + "github.com/soypat/lneto/ethernet" +) + +// TestHandlerRetransmitsAfterRTO covers the seam between a Handler and its +// LossRecovery, which the RTO unit tests do not: a lost data segment must be +// resent once the timer expires, with nothing arriving to prompt it. +func TestHandlerRetransmitsAfterRTO(t *testing.T) { + const mtu = ethernet.MaxMTU + const maxpackets = 4 + rng := rand.New(rand.NewSource(5)) + client, server := newHandler(t, mtu, maxpackets), newHandler(t, mtu, maxpackets) + + var now int64 // injected monotonic clock, in nanoseconds + client.SetLossRecovery(new(RTO), func() int64 { return now }) + + setupClientServer(t, rng, client, server) + var rawbuf [mtu]byte + establish(t, client, server, rawbuf[:]) + + data := []byte("hello") + if n, err := client.Write(data); err != nil || n != len(data) { + t.Fatal("client write:", n, err) + } + clear(rawbuf[:]) + n, err := client.Send(rawbuf[:]) + if err != nil || n == 0 { + t.Fatal("client send:", n, err) + } + // That frame is lost: it is never handed to the server. + + // Nothing may come back before the timer expires. + var probe [mtu]byte + if n, err := client.Send(probe[:]); err != nil || n != 0 { + t.Fatalf("client sent %d bytes before the RTO expired (err %v)", n, err) + } + + now += int64(3 * time.Second) // past the initial RTO and one backoff + + clear(probe[:]) + n, err = client.Send(probe[:]) + if err != nil { + t.Fatal("client send after RTO:", err) + } + if n == 0 { + t.Fatal("no retransmission after the RTO expired: the loss-recovery directive is never applied") + } + if err := server.Recv(probe[:n]); err != nil { + t.Fatal("server refused the retransmission:", err) + } + got := make([]byte, 16) + nr, err := server.Read(got) + if err != nil || string(got[:nr]) != string(data) { + t.Fatalf("server read %q (%v), want %q", got[:nr], err, data) + } +} + +// TestHandlerRetransmitsAfterCloseWithUnackedData is the write-then-close case +// every server performs. With the last data segment lost, the FIN behind it sits +// above a gap the peer cannot cross, so FIN-WAIT-1 must still retransmit that +// data or both sides wait forever. +func TestHandlerRetransmitsAfterCloseWithUnackedData(t *testing.T) { + const mtu = ethernet.MaxMTU + const maxpackets = 4 + rng := rand.New(rand.NewSource(9)) + client, server := newHandler(t, mtu, maxpackets), newHandler(t, mtu, maxpackets) + + var now int64 + client.SetLossRecovery(new(RTO), func() int64 { return now }) + + setupClientServer(t, rng, client, server) + var rawbuf [mtu]byte + establish(t, client, server, rawbuf[:]) + + data := []byte("last response bytes") + if n, err := client.Write(data); err != nil || n != len(data) { + t.Fatal("client write:", n, err) + } + clear(rawbuf[:]) + n, err := client.Send(rawbuf[:]) // this frame is lost in transit + if err != nil || n == 0 { + t.Fatal("client send:", n, err) + } + + // The application closes right after writing. + if err := client.Close(); err != nil { + t.Fatal("client close:", err) + } + var finbuf [mtu]byte + nfin, err := client.Send(finbuf[:]) // FIN (also lost, or simply unacked) + if err != nil { + t.Fatal("client send FIN:", err) + } + t.Logf("state after close: %s (FIN frame %d bytes)", client.State(), nfin) + + now += int64(3 * time.Second) // past the RTO + + var probe [mtu]byte + n, err = client.Send(probe[:]) + if err != nil { + t.Fatal("client send after RTO:", err) + } + if n == 0 { + t.Fatalf("no retransmission in %s: unacknowledged data is stranded by the close", client.State()) + } + if err := server.Recv(probe[:n]); err != nil { + t.Fatal("server refused the retransmission:", err) + } + got := make([]byte, 32) + nr, err := server.Read(got) + if err != nil || string(got[:nr]) != string(data) { + t.Fatalf("server read %q (%v), want %q", got[:nr], err, data) + } +}