Cover the decline path in the teardown drain and the log - #29
Merged
Conversation
The 0.6.0 drain waited on pjsua_call_get_count(), whose documentation says it includes "calls that are no longer active but still in the process of hanging up". That is true of a BYE and false of a decline: PJSUA disconnects the invite session the moment a final response is sent and releases the call slot, so the count is back to zero while the 603 has not been acknowledged. logout() drained nothing and deleted the account immediately — the exact behaviour 0.6.0 claimed to fix. The doc comment asserting otherwise was extrapolated from the pjsua_call_hangup docs, which are about BYE, and never checked. Measured rather than argued this time: the new test declines a real INVITE over loopback UDP and asserts pjsua_call_get_count() == 0 while the 603 is outstanding. Final responses are now tracked by Call-ID and CSeq from the transport hand-off until the ACK, and the drain waits on that alongside PJSUA's count. Entries expire after SIP timer H so one PBX that never ACKs cannot make every later teardown wait the full timeout. The observability moves for the same reason. on_call_tsx_state never fired for a declined call — PJSUA has nothing left to report it against. on_tsx_state cannot replace it either: sip_module.h says it reaches only the module "acting as transaction user", which for an INVITE is the invite session, never an application module. What every registered module does see is the response going out and the request coming in, so the observer watches those, registered ahead of the transaction layer because that is where the ACK to a non-2xx is absorbed. The send line now carries the destination, which is the thing a host could otherwise only get from a packet capture: it separates "nothing was sent" from "it was sent to that address and lost". The end-to-end test is the point of this change. It proves, without a PBX, that the 603 reaches the wire, that it is held until acknowledged, and that the ACK releases it.
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.
Reported from the field against 0.6.0: a never-answered call declined with
endCall(uuid:)leaves the PBX ringing, with no drain log and no transactionlog at all. The report's diagnosis was right on both counts.
The drain missed this path. It waited on
pjsua_call_get_count(), whosedocumentation says it includes "calls that are no longer active but still in the
process of hanging up". That describes a BYE. For a decline, PJSUA disconnects
the invite session as soon as the final response is sent and releases the call
slot, so the count is already zero while the
603is unacknowledged — the drainreturned instantly and the account was deleted underneath the response. Now
asserted by a test rather than reasoned about.
The observability missed it for the same reason, and the suggested fix could
not have worked:
sip_module.hsayson_tsx_stateis called "when this moduleis acting as transaction user for the specified transaction", which for an
INVITE is the invite session, not an application module. What every registered
module does see is
on_tx_responseandon_rx_request, so the observer watchesthe final response leaving and the ACK arriving, registered ahead of the
transaction layer because that is where the ACK to a non-2xx is absorbed.
The new send line carries the destination —
603 sent to <host>:<port> for Call-ID …— which is what separates "nothing was sent" from "sent there andlost" without a packet capture.
Test.
CallWaveDeclineTeardownTestsdrives a real INVITE at the engine's ownUDP transport over loopback, declines it, and asserts the
603comes back on thesocket, that it is held until acknowledged, and that the ACK releases it. It also
pins
pjsua_call_get_count() == 0at that moment, so if PJSUA ever changes, theextra tracking can go.
Note for the reporter: this test shows the
603does reach the wire, so theaccount-deletion race is unlikely to be the whole story behind the field
failure. See the PR discussion for what to capture next.