From c4a23703d9f0542d304ed6609951e9f9f7b9b06c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 20:43:23 +0000 Subject: [PATCH] fix(transaction): non-INVITE client transaction waits Timer K, not Timer D MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client transaction lingers in Completed to absorb retransmitted final responses. RFC 3261 17.1.1.2 gives an INVITE client transaction Timer D — 32 seconds on unreliable transports; 17.1.2.2 gives a non-INVITE one Timer K, which is T4. Init() set timer_d_time to Timer_D for both, so every OPTIONS, BYE, INFO and REGISTER transaction stayed in the transaction layer roughly six times longer than the RFC asks for. The behaviour is not wrong on the wire — the transaction simply absorbs retransmissions longer than it has to — but the table grows: a proxy or B2BUA that keep-alives many peers with OPTIONS holds thousands of terminated-but-not-removed transactions. Timer_K was already declared and populated by SetTimers, and was the only timer of the set never read. TestClientTxCompletedWaitPerMethod covers both kinds. --- sip/transaction_client_tx.go | 14 ++++++-- sip/transaction_client_tx_test.go | 53 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/sip/transaction_client_tx.go b/sip/transaction_client_tx.go index bf9b61a..0c05d8c 100644 --- a/sip/transaction_client_tx.go +++ b/sip/transaction_client_tx.go @@ -59,8 +59,18 @@ func (tx *ClientTx) Init() error { tx.timer_a = time.AfterFunc(tx.timer_a_time, func() { tx.spinFsm(client_input_timer_a) }) - // Timer D is set to 32 seconds for unreliable transports - tx.timer_d_time = Timer_D + // How long the transaction stays in Completed absorbing retransmitted + // final responses. RFC 3261 17.1.1.2 gives an INVITE client transaction + // Timer D (32 seconds for unreliable transports); 17.1.2.2 gives a + // non-INVITE one Timer K (T4). Using Timer D for both keeps every + // OPTIONS, BYE and INFO transaction in the layer six times longer than + // the RFC asks for, which shows as transaction table growth on a proxy + // or B2BUA that sends keep-alive OPTIONS to many peers. + if tx.origin.IsInvite() { + tx.timer_d_time = Timer_D + } else { + tx.timer_d_time = Timer_K + } tx.mu.Unlock() } diff --git a/sip/transaction_client_tx_test.go b/sip/transaction_client_tx_test.go index 5ce31d4..eae0432 100644 --- a/sip/transaction_client_tx_test.go +++ b/sip/transaction_client_tx_test.go @@ -129,3 +129,56 @@ func TestClientTransactionFSM(t *testing.T) { require.NoError(t, compareFunctions(tx.currentFsmState(), tx.inviteStateAccepted)) }) } + +// A client transaction lingers in Completed to absorb retransmitted final +// responses, and RFC 3261 gives the two transaction kinds different timers for +// that wait: 17.1.1.2 gives an INVITE one Timer D (32 seconds on unreliable +// transports), 17.1.2.2 gives a non-INVITE one Timer K (T4). Using Timer D for +// both keeps every OPTIONS, BYE and INFO in the transaction layer six times +// longer than needed. +// +// On a reliable transport both waits are zero, and this checks that too: the +// timer is picked in the unreliable branch only, and a change there must not +// start arming a timer over TCP. +func TestClientTxCompletedWaitPerMethod(t *testing.T) { + SetTimers(1*time.Millisecond, 1*time.Millisecond, 1*time.Millisecond) + + completedWait := func(t *testing.T, req *Request) time.Duration { + t.Helper() + req.raddr = Addr{IP: net.ParseIP("127.0.0.99"), Port: 5060} + conn := &UDPConnection{ + PacketConn: &fakes.UDPConn{ + Reader: bytes.NewBuffer([]byte{}), + Writers: map[string]io.Writer{"127.0.0.99:5060": bytes.NewBuffer([]byte{})}, + }, + } + tx := NewClientTx("timer-k", req, conn, slog.Default()) + require.NoError(t, tx.Init()) + tx.mu.Lock() + defer tx.mu.Unlock() + return tx.timer_d_time + } + + invite := func(transport string) *Request { + req, _, _ := testCreateInvite(t, "sip:127.0.0.99:5060", transport, "127.0.0.2:5060") + return req + } + options := func(transport string) *Request { + return testCreateRequest(t, "OPTIONS", "sip:127.0.0.99:5060", transport, "127.0.0.2:5060") + } + + for _, tc := range []struct { + name string + req *Request + want time.Duration + }{ + {"INVITE over UDP waits Timer D", invite("udp"), Timer_D}, + {"OPTIONS over UDP waits Timer K", options("udp"), Timer_K}, + {"INVITE over TCP does not wait", invite("tcp"), 0}, + {"OPTIONS over TCP does not wait", options("tcp"), 0}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, completedWait(t, tc.req)) + }) + } +}