Repository navigation
fix(transaction): end the server Accepted state on the matching ACK (review follow-up to #164) - #169
Merged
Merged
Conversation
Follow-up review of #164 found that a confirmed server dialog left the Accepted transaction parked in the endpoint's table until Timer L (64*T1): nobody drains it after the dialog's receive loop breaks, and the process_timer fallback detaches with detach(key, None), which does not clean waiting_ack — so every confirmed call also leaked a waiting_ack entry for the process lifetime. - a matching ACK now ends the Accepted state immediately (Terminated): Timer L/G are cancelled, waiting_ack is removed, and the 2xx stays cached in finished_transactions so retransmitted ACKs and late INVITE retransmissions are absorbed below the TU (the pre-#164 post-ACK behavior) - handle_reinvite (InviteDialog + ServerInviteDialog) tracks 'acked' explicitly and passes answered_2xx && !acked to end_session_without_ack, so the timeout path cannot fire on a confirmed re-INVITE now that the ACK ends the transaction - drop the dead 2xx-suppression guard in the Completed Timer G arm (server 2xx never routes to Completed) - new test_accepted_lifecycle suite: server tables are clean right after the ACK; the client Accepted transaction detaches exactly at Timer M with the ACK kept cached
This was referenced Oct 6, 2026
shenjinti
added a commit
that referenced
this pull request
Oct 6, 2026
Ships this round on top of 0.7.0: proxy-mode auto_ack_2xx (#146/#172), ACK CSeq routing (#155/#170/#172), digest auth_username (#153), dead stream retirement (#161), remote-ack getter (#157), UAS ACK timeout → BYE (#149/#169), RFC 6026 Accepted state + Timer L/M with documented deviations (#164/#169), CANCEL/2xx race handling (#162/#171), in-dialog Via transport (#163), flow-reuse and ACK hardening tests.
This was referenced Oct 7, 2026
shenjinti
pushed a commit
that referenced
this pull request
Oct 7, 2026
… ends it Since #169 the matching ACK terminates an Accepted server INVITE transaction, and cleanup() took last_response before the dialog built DialogState::Confirmed from it. Confirmed carried Response::default() (no CSeq) for the initial INVITE and every re-INVITE, so a TU could not tell which INVITE it confirms. The server INVITE transaction now keeps last_response and hands a copy to finished_transactions.
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
1 participant
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.
Follow-up review of #164 (RFC 6026 adoption).
Bug found: after a server dialog was confirmed by the ACK, the Accepted transaction stayed in
endpoint.transactionsuntil Timer L (64*T1) because nothing drains it once the dialog's receive loop breaks. Theprocess_timerfallback detaches withdetach(key, None), which does not cleanwaiting_ack— so every confirmed call leaked awaiting_ackentry for the process lifetime, on top of the 32s table residency.Fix: a matching ACK ends the Accepted state immediately. Retransmitted ACKs and late INVITE retransmissions are still absorbed below the TU via
finished_transactions(identical to the pre-#164 post-ACK behavior).handle_reinvitenow tracksackedexplicitly so the no-ACK timeout path cannot fire on a confirmed re-INVITE. Also drops a provably-dead defensive branch.Verification: new
test_accepted_lifecyclesuite asserts server tables are clean immediately after the ACK and the client Accepted transaction detaches exactly at Timer M (with the ACK cached); 377 lib + 65 doc tests green across repeated runs, fmt clean, clippy at the main baseline.