[kulcsartibor]

Two small fixes found while running a credit-control server on v4.5.x with MaxConcurrentHandlers and a DRA in front of it. One commit each, each with a regression test that fails on current main. About ten lines of non-test code in total; no new API.

1. The peer's first request after the CEA can be dropped under concurrent dispatch

With Server.MaxConcurrentHandlers set, every message is handled on its own goroutine, CER included. handleCER wrote the CEA first and stored the peer metadata on the connection afterwards (c.SetContext). A peer that sends its first request as soon as it reads the CEA can have that request reach handshakeOK before the CER goroutine reaches SetContext, and handshakeOK drops it silently: no answer, no error report. The peer sees a Tx timeout on its first request.

Reproduced on the wire (v4.5.1): 3,000 handshakes each followed immediately by a CCR, GOMAXPROCS=2, CPU load from four busy loops.

Dispatch	Unanswered first requests
MaxConcurrentHandlers=8	2 / 3,000 (the CCR handler ran 2,998 times)
MaxConcurrentHandlers=8, with a 50 ms sleep inserted between the CEA write and SetContext	20 / 20
sequential (0)	0 / 3,000

After the fix, the first run shows 0 / 3,000 under the same load. Present since MaxConcurrentHandlers was added in v4.2.0.

Fix: handleCER sets the context before successCEA writes the CEA. A request can only arrive after the peer has read the CEA, and by then the context is set. A failed CEA write now closes the connection; previously it was reported and the connection left open.

Test: TestHandleCER_PeerContextSetBeforeCEA. The OnCEA hook runs immediately before the CEA write, so a hook that checks smpeer.FromContext(c.Context()) observes the order deterministically, with no sleeps or load.

Not covered: two CERs dispatched concurrently (a retransmission) can both get a CEA. Handling CER/DWR/DPR inline in conn.dispatch would fix that too; I've left it out of this PR because it changes dispatch semantics in diam, and can propose it separately if you want it.

2. A failed write leaves the connection open with a writer that never recovers

Conn.Write goes through a bufio.Writer, which keeps returning its first error from every later Write and Flush (crypto/tls behaves the same). writeLocked only returned the error. So after one write that hit Server.WriteTimeout — typically a DRA that stopped reading for a while — the connection stayed open: the read loop kept reading, handlers kept processing requests, and every answer, DWA and server-initiated request failed, until the peer's watchdog gave up.

Reproduced (v4.5.1): a handler floods a non-reading peer until a 200 ms write timeout, the peer drains everything, the handler writes one small message: still i/o timeout. The peer's next read then timed out rather than returning EOF, so the server still held the connection open.

Fix: writeLocked closes the underlying connection on any write error, on both the buffered TCP path and the SCTP path. The read loop then exits on its read error, serve's deferred close is idempotent, CloseNotify fires, and the peer can fail over. Resetting the writer would not do, because a partial frame may already be on the wire. The client role already closes on its own failed writes in sm/client.go; this covers the server's own CEA/DWA writes and every application write.

Behaviour change: a write that times out now ends the connection instead of leaving it open but unusable. Anyone running a very low WriteTimeout will see a disconnect where they previously saw silent failure.

Test: TestWriteErrorClosesConn floods a non-reading peer until a write times out and asserts the peer then reads EOF.

[fiorix]

Thanks, Tibor. Both fixes are merged as two commits, and the reproductions and the deterministic OnCEA test made them easy to review.

Remainder, found by the agent:

One follow-up on main: writeLocked no longer closes the connection on an SCTP write error. For TCP and TLS the close is right for the reason you gave, since bufio keeps returning its first error and a partial frame may be on the wire. SCTP sends whole messages, so a failed send leaves nothing partial, and errors such as EMSGSIZE or an invalid stream only affect that message. Closing on those would drop the association and everything in flight on it. Message.WriteTo on SCTP goes through WriteStream and never reached that path anyway, and a dead association still fails the reads and closes.