fix(transaction): add the 2xx ACK route before sending the 2xx

[tgeorge06]

Fixes #192.

Thanks to @beija-wq for the report. The analysis in the issue is exact, and the reproduction program made it easy to measure before and after.

Problem

A server INVITE transaction adds the route that a 2xx ACK takes (a new branch, matched by dialog and CSeq in waiting_ack_cseq) only when it enters Accepted, and respond() enters Accepted only after the 2xx has been sent. A peer on the same host or LAN can ACK the 2xx in between. That ACK matches no transaction and is dropped without a log line.

Expected

RFC 6026 §7.1 (updating RFC 3261 §17.2.1): a server INVITE transaction enters "Accepted" when the TU passes it a 2xx, and

Any ACKs received from the network while in the "Accepted" state MUST be passed directly to the TU and not absorbed.

RFC 3261 §13.3.1.4:

The 2xx response is passed to the transport with an interval that starts at T1 seconds and doubles for each retransmission until it reaches T2 seconds (T1 and T2 are defined in Section 17). Response retransmissions cease when an ACK request for the response is received.

An ACK that arrives right after the 2xx left the socket is "received while in Accepted" from the peer's point of view and must reach the TU.

What happens

Line numbers at current main (b47f434):

• Transaction::respond sends first and transitions afterwards (src/transaction/transaction.rs:499-500).
• The waiting_ack_cseq route (and the dialog-only waiting_ack entry) is inserted inside transition(Accepted) (transaction.rs:1366-1373).
• EndpointInner::on_received_message rewrites a 2xx ACK's key only when that route exists (src/transaction/endpoint.rs:384-396). Otherwise the ACK's own key (new branch) matches no transaction (endpoint.rs:491) and Method::Ack => return Ok(()) drops it (endpoint.rs:528).

What a lost ACK costs:

• A UAC that ACKs every 2xx retransmission (RFC 3261 §13.2.2.4) recovers: Timer G resends the 2xx after T1 on every transport (the documented Timer G deviation, transaction.rs:1190-1211), the UAC ACKs it again, and the dialog is Confirmed about T1 (500 ms) late. In the measurement below, every one of the 10 lost ACKs was recovered this way, 500 ms after the 2xx.
• A UAC that ACKs only once, or whose re-ACK is lost too, never confirms the dialog. Timer L ends the transaction at 64*T1, and the dialog ends the session with a BYE and TerminatedReason::Timeout (src/dialog/dialog.rs:1415, pinned by test_unacked_2xx_is_retransmitted_then_the_session_is_ended). The same applies to a re-INVITE: an answered re-INVITE whose ACK was dropped ends the call.

The window is transport independent: the route is missing for UDP, TCP, TLS and WS alike.

A non-2xx final has no such window: its ACK carries the INVITE's branch (§17.1.1.3), so it matches the transaction key itself, which is attached from the start, and waits in the transaction's queue until respond() returns and the transaction is in Completed.

Fix

respond() adds the waiting_ack_cseq route before the send, when the 2xx moves the transaction into Accepted (not for a 2xx retransmitted by the TU while already in Accepted, which has the route). An ACK that arrives during the send is now routed to the transaction's queue and handled by receive() after the transition, as any ACK in Accepted is today.

A small drop guard removes the route again if the send fails or the respond() future is dropped (for example by the t1x64 timeout around tx.respond() in process_transaction_handle, dialog.rs:1687, which then sends a 501 on the same transaction). It removes the entry only while it still maps to this transaction, and is disarmed once the send has completed. transition(Accepted) still adds the route (now through the shared add_ack_cseq_route helper, a no-op the second time) and the dialog-only waiting_ack entry, as before.

+49 / -9 lines in src/transaction/transaction.rs, most of it the guard and comments.

Unchanged on purpose:

• The ACK CSeq check in Accepted and Completed (fix: ignore an ACK whose CSeq does not match the server INVITE #155), an ACK ending an Accepted server INVITE (fix(transaction): end the server Accepted state on the matching ACK (review follow-up to #164) #169), routing by (dialog, CSeq) (fix: route the ACK of a 2xx to the INVITE with the same CSeq (rebase of #167) #170) and the 2xx kept in finished_transactions (fix(transaction): keep a server INVITE's final response after the ACK ends it #174) behave exactly as before; the ACK takes the same path, only the route exists earlier.
• Non-2xx finals, provisional responses and non-INVITE server transactions take no new code path.
• The dialog-only waiting_ack map is still filled only by the transition, so EndpointStats::waiting_ack counts what it counted before.

Contract / coverage

What a server INVITE transaction does with the ACK of its final response:

final	ACK arrives	before	after
2xx	after respond() returned	delivered, Terminated	same
2xx	while the 2xx is being sent	dropped; Confirmed only via a re-ACK after T1, else BYE at 64*T1	delivered, Terminated
3xx-6xx	either	delivered, Confirmed	same

respond() with a 2xx	waiting_ack_cseq afterwards
send succeeds	route to this transaction (as before)
send fails	no route (as before)
future dropped during the send	no route (as before)

Tests

src/transaction/tests/test_server_invite_ack.rs: test_server_invite_ack_during_final_response_send. The server transaction sends over a channel transport whose bounded queue is full, so the send waits; the test polls reply() once (pending inside the send), delivers the ACK through EndpointInner::on_received_message, then frees the queue and lets reply() finish. No timing is involved. Rows:

• 2xx, new-branch ACK during the send: delivered by receive(), transaction Terminated, no route left;
• 486, INVITE-branch ACK during the send: delivered, Confirmed (already worked, a control);
• 2xx whose send fails: reply() errors, still Trying, no route;
• 2xx whose reply() future is dropped during the send: still Trying, no route.

On main (b47f434) it fails:

---- transaction::tests::test_server_invite_ack::test_server_invite_ack_during_final_response_send stdout ----
panicked at src/transaction/tests/test_server_invite_ack.rs:343:33:
200 OK: the ACK must reach the transaction

The non-2xx, send-failure and cancellation rows pass on main. With the fix, removing the guard's cleanup makes the send-failure and cancellation rows fail (waiting_ack_cseq.len() is 1).

Load, with the reproduction program from the issue (UDP over 127.0.0.1, default EndpointOption, a raw peer that ACKs each 200 once; "lost" = not Confirmed within 400 ms). macOS, 16 CPUs, so the window is hit less often than on the reporter's 4-CPU Linux box:

setting	main	this PR
3x300, 1 at a time	0/900	0/900
3x300, 10 at a time	1/900	0/900
3x1000, 50 at a time	0/3000	0/3000
5x1000, 50 at a time, 16 worker threads	1/5000	0/5000
25x1000, 50 at a time, 4 worker threads	4/25000	0/25000
10x1000, 50 at a time, 4 worker threads, peer re-ACKs every 2xx	10/10000, all Confirmed 500 ms after the 2xx	0/10000

Checks

• cargo test --features bench: 413 lib tests passed, 0 failed (412 on main plus the new one), and 65 doc tests passed. Plain cargo test passes too. The new test passed 20 runs in a row.
• cargo check --no-default-features --features platform-embassy: the same output as on main.
• cargo clippy --features bench --all-targets: the same output as on main, none in the changed code. (On main it stops at a clippy::never_loop error in src/dialog/tests/test_refer_notify.rs:98, unrelated to this PR; with -A clippy::never_loop the warnings are the same as on main.)
• rustfmt --check on the changed files: clean.

[beija-wq]

Thanks a lot @tgeorge06 for picking this up so quickly, and for the clear write-up and the deterministic test.

I confirm the fix matches what we observed: on our side (Linux, 4 vCPU, UDP on the LAN) the ACK of the 200 OK was occasionally dropped in exactly that window, and the dialog only got confirmed after a 2xx retransmission. We currently work around it in our driver; once this lands in a release we'll drop the workaround and keep our regression test against it.