diff options
author | mmichelson <mmichelson@f38db490-d61c-443f-a65b-d21fe96a405b> | 2009-07-10 17:39:13 +0000 |
---|---|---|
committer | mmichelson <mmichelson@f38db490-d61c-443f-a65b-d21fe96a405b> | 2009-07-10 17:39:13 +0000 |
commit | 69996e29bdaa7402bd27b0fb33088e4060921c97 (patch) | |
tree | a9b3bdf159a3ea2f3f12dbe3a69d98e08aac76d9 /channels | |
parent | ab118f87727c8f9c0ccd510ee47c7fd8ef6d0b3a (diff) |
Properly ACK 487 responses to canceled INVITEs.
From the review board request:
The fix from review 298 has exposed a new bug in chan_sip.
When we hang up an outgoing call, we first will dump all the outstanding
packets on the sip_pvt using __sip_pretend_ack. Then, if we can, we send
a CANCEL. The problem with this is that since destroyed all the outstanding
packets on the dialog, we cannot match the incoming 487 response to our
INVITE. Because we cannot match the response, we do not send an ACK.
To correct this, instead of using __sip_pretend_ack, I have changed the code
to loop through the list of packets and call __sip_semi_ack on each one
instead. This causes us to stop retransmitting the requests, but we still have
them around in case we get responses for them.
When discussing this earlier today with Josh Colp, we both agreed that in the
majority of cases, this would be enough of a fix. However, we also agreed that
we should have a safety net in place in case we never receive a response to
our initial INVITE. To handle this, I have modified __sip_autodestruct to
behave similar to the way it does in Asterisk 1.4. If there are outstanding
packets on the sip_pvt, the needdestroy flag is not set, and the last request
on the dialog was either a CANCEL or BYE, then we set the needdestroy flag and
reschedule destruction for 10 seconds in the future. If, though, the
needdestroy flag is set, then we use __sip_pretend_ack to kill the remaining
outstanding packets so that the monitor thread can destroy the sip_pvt.
I ran two separate tests. First, I placed a call from my Aastra phone to my
Polycom phone. I hung up the Aastra before the Polycom answered. I verified
through sip debug output that Asterisk properly ACKed the 487 received from the
Polycom.
For my second test, I set up a SIPp UAS scenario so that it would send a 200 OK
in response to a CANCEL but would not send a 487 for the original INVITE. I
verified that after about 40 seconds, Asterisk properly cleans up the outgoing
sip_pvt for the call.
Review: https://reviewboard.asterisk.org/r/308
git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@205877 f38db490-d61c-443f-a65b-d21fe96a405b
Diffstat (limited to 'channels')
-rw-r--r-- | channels/chan_sip.c | 12 |
1 files changed, 9 insertions, 3 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 78c39a1c3..0c403ea2c 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -2120,11 +2120,14 @@ static int __sip_autodestruct(const void *data) * that were created via INVITE, then thru some sequence were CANCELED, * to die, rather than infinitely be rescheduled */ if (p->packets && !ast_test_flag(&p->flags[0], SIP_NEEDDESTROY)) { + char method_str[30]; if (option_debug > 2) ast_log(LOG_DEBUG, "Re-scheduled destruction of SIP call %s\n", p->callid ? p->callid : "<unknown>"); append_history(p, "ReliableXmit", "timeout"); - if (p->method == SIP_CANCEL || p->method == SIP_BYE) { - ast_set_flag(&p->flags[0], SIP_NEEDDESTROY); + if (sscanf(p->lastmsg, "Tx: %s", method_str) == 1 || sscanf(p->lastmsg, "Rx: %s", method_str) == 1) { + if (method_match(SIP_CANCEL, method_str) || method_match(SIP_BYE, method_str)) { + ast_set_flag(&p->flags[0], SIP_NEEDDESTROY); + } } return 10000; } @@ -3665,7 +3668,10 @@ static int sip_hangup(struct ast_channel *ast) if (needcancel) { /* Outgoing call, not up */ if (ast_test_flag(&p->flags[0], SIP_OUTGOING)) { /* stop retransmitting an INVITE that has not received a response */ - __sip_pretend_ack(p); + struct sip_pkt *cur; + for (cur = p->packets; cur; cur = cur->next) { + __sip_semi_ack(p, cur->seqno, ast_test_flag(cur, FLAG_RESPONSE), cur->method ? cur->method : find_sip_method(cur->data)); + } /* if we can't send right now, mark it pending */ if (p->invitestate == INV_CALLING) { |