[fridokus]

The bug

connectionPool.Delete and CloseAndDelete remove an entry by address alone, without
checking that the entry still names the connection being removed:

func (p *connectionPool) CloseAndDelete(c Connection, addr string) error {
	p.Lock()
	defer p.Unlock()
	delete(p.m, addr)   // <- deletes whatever is under addr, not necessarily c
	...
}

An address is only a name for a connection, and names get reused. A peer that reconnects
from the same source port produces a successor that Add files under a key its predecessor
still holds, and the successor wins it — which is the intended outcome, since the newer
connection is the reachable one. Add's own TODO how about multi connection support for same remote address is about this situation.

The predecessor's reader goroutine then exits, as it must, and runs its cleanup:

defer t.pool.Delete(laddr)
defer func() { t.pool.CloseAndDelete(conn, raddr) }()

Both names now belong to the successor, so the dying reader evicts a connection it has
nothing to do with. The successor stays open and stays read — only the pool forgets it, so
nothing re-adds it, because nothing knows it is gone. Every subsequent message routed to
that address either dials a second connection or fails outright.

It is easiest to hit with SIP over TCP, where a UA re-registering from the same source port
is routine, and it is worst there too: the orphaned connection is the one the far end is
still listening on, so requests sent the other way are lost. We found it in a P-CSCF, where
TS 33.203 makes the re-registration case unavoidable — the UE keeps port_us across a
re-registration, so every refresh produces exactly this predecessor/successor pair on a
shared remote key.

The fix

Delete an entry only while it still names the connection being removed.

connectionPool is package-private, so taking the connection as a parameter on Delete and
DeleteMultiple changes no exported API. Every caller is a reader-exit path that already
has the connection in hand (transport_tcp.go, transport_ws.go, transport_udp.go), so
each was a one-word change.

Tests

Two new tests in sip/transport_connection_pool_successor_test.go, verified by mutation
rather than by coverage:

mutation	result
deleteIfCurrent deletes unconditionally (i.e. the bug restored)	TestAPredecessorDoesNotEvictItsSuccessor FAIL, control PASS
deleteIfCurrent never deletes (i.e. a "fix" that leaks)	TestASoleConnectionIsStillDeleted FAIL, other PASS
none	both PASS

The second exists so the first cannot be satisfied by a change that simply stops deleting,
which would turn the pool into a leak.

go test ./sip/ is green. go test ./... fails to build the root package on
pattern testdata/certs/client.crt: no matching files found, which is identical before and
after this change.

gofmt -l reports sip/request.go, also pre-existing and untouched here.

[emiago]

thanks for reporting, fixed on main