[Home]

Summary:ASTERISK-14653: [patch] chan_sip fails to destroy channels in INVITE when no response received
Reporter:dant (dant)Labels:
Date Opened:2009-08-13 19:15:56Date Closed:2009-12-10 10:57:47.000-0600
Priority:MinorRegression?Yes
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip.c.diff
Description:In the event that a call is made to a device that is not responding INVITE packets are reliably transmitted to the device, if the call is hungup the channel lives forever.


****** STEPS TO REPRODUCE ******

Add a static peer with a local IP address that is not in use.
Make a call to that peer.
End the call.
Check show sip channels.

****** ADDITIONAL INFORMATION ******

The SIP specs (rfc3261) state that CANCEL should only be sent if a response has been received and that the UA should wait for a response before sending a CANCEL but that if no response has been received in 32 seconds then the UA should consider the request cancelled. When the call ends asterisk schedules the channel for destruction in 32 seconds, however, __sip_autodestruct in the event that packets are still waiting for delivery checks to see if needdestroy is set, and if not, checks to see if the lastmsg was a Tx or Rx of CANCEL or BYE before setting needdestroy, otherwise it just reschedules destruction by returning 10000. The result for a channel with lastmsg of Init: INVITE is that it will live for ever being forever rescheduled for destruction.
Comments:By: dant (dant) 2009-08-13 19:21:21

The attached patch modifies __sip_autodestruct to also set needdestroy in the event that lastmsg was Init: INVITE. _sip_autodestruct and this changed code will be called 32 seconds after the call has ended as scheduled then needdestroy will be set and after a further 32 seconds __sip_autodestruct will be run again and as needdestroy is set, call __sip_pretend_ack and carry on with actually destroying the dialog.
By: Private Name (falves11) 2009-08-16 01:36:16

I tested the patches supplied by the reporter in versions 1.4 and 1.6.2 and they work fine. This patch should go right in ASAP.
By: Leif Madsen (lmadsen) 2009-08-17 08:06:36

Assigned to Mark for now since he can probably review quickly. However, I know Mark has been busy lately, so he may choose to unassign himself for now. Thanks!
By: Mark Michelson (mmichelson) 2009-08-17 09:53:35

I'm a bit confused here. I'm having a hard time seeing why this is necessary.

We have logic built in so that if we are getting no response to the INVITE at all, we will stop retransmitting after a certain period. You'll see a message like "Maximum retries exceeded on..." When that happens, the channel should get hung up. If it is not, then there is some other bug that should be handled a different way.

And of course, if we have gotten a response, the logic in the sip_hangup function should cause a CANCEL to be sent out.

It would be helpful to see a console log to determine what is happening here. I have no doubt that your patch has fixed the problem, but it shouldn't be necessary since this is something that should be taken care of already in the code. I'm thinking that there must have been a regression to have caused this, and it would be great to figure out what that is and get it fixed instead of adding new code to work around the broken code.

Also, sorry to have to do this, but I am so bogged down with things I'm going to have to unassign this from myself.
By: dant (dant) 2009-08-18 03:20:44

The problem is not that it keeps retransmitting, the problem is that when the outgoing attempt is terminated the dialog is never actually destroyed though retransmissions are stopped and it is scheduled for destruction...

I think I did tag this as a regression...

Below are details of the change that appears to have caused this issue... The associated documentation suggests that this was done on purpose to be able to match an incoming 487 response, however, this code only hits after 32 seconds from the channel being scheduled for termination which is as long as we should be waiting for a response and the patch causes it to wait a further 10 seconds before setting needdestroy... The check for CANCEL or BYE before setting needdestroy was described as being the safety net for incase a response to INVITE was never received, but, there would never be a CANCEL or BYE in the normal situation where a response to INVITE was never received because the remote host didn't respond at all, hence, the safety net is broken and this patch corrects that...

As mentioned below, this has been applied to all branches, so all branches will now leak sockets build up an excessive supply of dialogs when dealing with non-responsive hosts being sent INVITEs...

r205877 | mmichelson | 2009-07-11 01:39:13 +0800 (Sat, 11 Jul 2009) | 40 lines

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
By: Private Name (falves11) 2009-08-18 05:34:29

I hope that this patch gets approved since it really works and without it Asterisk starts accumulating UDP sockets until the whole server goes down. This issue ruined my month-long vacation in Europe.
By: Mark Michelson (mmichelson) 2009-08-18 15:37:02

dant: I think I get what you're saying now. Basically, we try to send the INVITE the maximum number of times, but the INVITE doesn't actually get removed from the list of packets on the sip_pvt. This used to be taken care of in sip_hangup because the packets were all cleared out prior to sending the CANCEL.

So, there are two methods for handling this. One is the way you have done it, and the other way would be to modify retrans_pkt so that when the packet has reached the maximum number of retries, the packet is removed from the list of packets on the sip_pvt. Personally, I'm more in favor of the second method since it will handle any reliably-transmitted outbound request type and not just INVITEs. Do you agree with this, or can you see a potential flaw in my reasoning?

falves11: I already established in my first note that I acknowledge that the patch contributed will work. You don't seem to understand that just because a patch will make the issue go away, that it is not necessarily the most correct way to handle the problem. Also, please leave drama about how an issue "ruined" your vacation elsewhere. It does not help anything at all ever.
By: dant (dant) 2009-08-18 23:26:41

Sorry for not being too clear...

I'd not considered termination due to maximum retries, though this would likely produce the same effect... the vector that I was targetting was when a call setup was terminated prior to maximum retries being reached... i.e. call a non-responding peer and hangup before receiving a response and before having reached maximum retries...

I suspect that modifying retrans_pkt wouldn't correct the behaviour in this situation as the dialog would be scheduled for destruction outside of retrans_pkt and as such the packet will still be there and sched will still be triggering __sip_autodestruct, but, I'll have a look and see if there's somewhere else this could be put... I originally went with this approach as the logic seemed to be that it would only actually kill off the dialog if a CANCEL or BYE had been sent, whereas another valid final method is INVITE if there has been no response as we shouldn't send a CANCEL in that situation.
By: Private Name (falves11) 2009-08-20 01:42:04

There is still an issue relate to this patch. Some ACK's are never killed like those INVTE's. For example, right now it is 2:30 AM and there has not been any calls since a few hours ago, but I see dozens of lines like this this when I type "sip show channels"
208.78.161.210 3864922293 790a13246b2 00103/00102 0x0 (nothing) No Tx: ACK

of course "core show channel"s shows nothing. Linux shows UDP ports still in use.
By: Ian Anderson (seadweller) 2009-09-10 23:47:27

EDIT:  It's late and I did not read enough, will try this patch against my ghost "INVITES" tomorrow.


By: kresike (kresike) 2009-09-28 09:51:53

I have also noticed this problem after upgrading to * version 1.4.26.
I tend to agree with mmichelson's second solution, (no offence dant).
Is there any progress on that ?
By: Private Name (falves11) 2009-09-28 11:02:24

I remember looking at the code and this patch is in fact in 1.4 SVN, since long ago. Am I wrong?
By: Leif Madsen (lmadsen) 2009-09-28 14:17:09

Well, someone could test latest 1.4 SVN to verify. I know a lot of security releases were done recently, so it is possible the bug fix didn't get in until 1.4.27-rc1.
By: kresike (kresike) 2009-09-30 03:40:43

I would be willing to try this, but cannot find the patch in 1.4 SVN or 1.4.27-rc1 either. It's probably just me looking in the wrong places. Could you give me an URL ???
By: Leif Madsen (lmadsen) 2009-12-08 09:51:24.000-0600

http://downloads.asterisk.org/pub/telephony/asterisk/

Use Asterisk 1.4.27.1
By: Digium Subversion (svnbot) 2009-12-10 10:08:32.000-0600

Repository: asterisk
Revision: 234095

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r234095 | tilghman | 2009-12-10 10:08:21 -0600 (Thu, 10 Dec 2009) | 9 lines

When we receive no response at all to our INVITE, allow the channel to be destroyed.
(closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
Reported by: falves11
Patches:
      20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
      20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
Tested by: falves11
Review: https://reviewboard.asterisk.org/r/446/

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234095
By: Digium Subversion (svnbot) 2009-12-10 10:22:32.000-0600

Repository: asterisk
Revision: 234095

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines

When we receive no response at all to our INVITE, allow the channel to be destroyed.
(closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
Reported by: falves11, corruptor, dant
Patches:
      20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
      20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
Tested by: falves11
Review: https://reviewboard.asterisk.org/r/446/

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234095
By: Digium Subversion (svnbot) 2009-12-10 10:24:34.000-0600

Repository: asterisk
Revision: 234129

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r234129 | tilghman | 2009-12-10 10:24:28 -0600 (Thu, 10 Dec 2009) | 16 lines

Merged revisions 234095 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
 
 When we receive no response at all to our INVITE, allow the channel to be destroyed.
 (closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
  Reported by: falves11, corruptor, dant
  Patches:
        20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
        20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
  Tested by: falves11
 Review: https://reviewboard.asterisk.org/r/446/
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234129
By: Digium Subversion (svnbot) 2009-12-10 10:30:31.000-0600

Repository: asterisk
Revision: 234131

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_sip.c

------------------------------------------------------------------------
r234131 | tilghman | 2009-12-10 10:30:25 -0600 (Thu, 10 Dec 2009) | 23 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
   
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
    Reported by: falves11, corruptor, dant
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234131
By: Digium Subversion (svnbot) 2009-12-10 10:30:46.000-0600

Repository: asterisk
Revision: 234132

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r234132 | tilghman | 2009-12-10 10:30:39 -0600 (Thu, 10 Dec 2009) | 23 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
   
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
    Reported by: falves11, corruptor, dant
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234132
By: Digium Subversion (svnbot) 2009-12-10 10:38:51.000-0600

Repository: asterisk
Revision: 234133

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_sip.c

------------------------------------------------------------------------
r234133 | tilghman | 2009-12-10 10:38:45 -0600 (Thu, 10 Dec 2009) | 23 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
   
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576, closes issue ASTERISK-14653, closes issue ASTERISK-15161, closes issue ASTERISK-14337, issue ASTERISK-14500)
    Reported by: falves11, corruptor, dant
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234133
By: Digium Subversion (svnbot) 2009-12-10 10:53:48.000-0600

Repository: asterisk
Revision: 234095

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 17 lines

When we receive no response at all to our INVITE, allow the channel to be destroyed.
(closes issue ASTERISK-14576)
Reported by: falves11
Patches:
      20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
      20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
Tested by: falves11
Review: https://reviewboard.asterisk.org/r/446/
(closes issue ASTERISK-14653)
Reported by: dant
(closes issue ASTERISK-15161)
Reported by: corruptor
(closes issue ASTERISK-14337)
Reported by: falves11
(issue ASTERISK-14500)
Reported by: lftsy

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234095
By: Digium Subversion (svnbot) 2009-12-10 10:55:16.000-0600

Repository: asterisk
Revision: 234129

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 24 lines

Merged revisions 234095 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
 
 When we receive no response at all to our INVITE, allow the channel to be destroyed.
 (closes issue ASTERISK-14576)
  Reported by: falves11
  Patches:
        20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
        20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
  Tested by: falves11
 Review: https://reviewboard.asterisk.org/r/446/
 (closes issue ASTERISK-14653)
 Reported by: dant
 (closes issue ASTERISK-15161)
 Reported by: corruptor
 (closes issue ASTERISK-14337)
 Reported by: falves11
 (issue ASTERISK-14500)
 Reported by: lftsy
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234129
By: Digium Subversion (svnbot) 2009-12-10 10:56:21.000-0600

Repository: asterisk
Revision: 234131

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_sip.c

------------------------------------------------------------------------
r234131 | tilghman | 2009-12-10 10:30:22 -0600 (Thu, 10 Dec 2009) | 29 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576)
    Reported by: falves11
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
   (closes issue ASTERISK-14653)
   Reported by: dant
   (closes issue ASTERISK-15161)
   Reported by: corruptor
   (closes issue ASTERISK-14337)
   Reported by: falves11
   (issue ASTERISK-14500)
   Reported by: lftsy
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234131
By: Digium Subversion (svnbot) 2009-12-10 10:57:14.000-0600

Repository: asterisk
Revision: 234132

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r234132 | tilghman | 2009-12-10 10:30:32 -0600 (Thu, 10 Dec 2009) | 31 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
   
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576)
    Reported by: falves11
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
   (closes issue ASTERISK-14653)
   Reported by: dant
   (closes issue ASTERISK-15161)
   Reported by: corruptor
   (closes issue ASTERISK-14337)
   Reported by: falves11
   (issue ASTERISK-14500)
   Reported by: lftsy
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234132
By: Digium Subversion (svnbot) 2009-12-10 10:57:44.000-0600

Repository: asterisk
Revision: 234133

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_sip.c

------------------------------------------------------------------------
r234133 | tilghman | 2009-12-10 10:30:40 -0600 (Thu, 10 Dec 2009) | 31 lines

Merged revisions 234129 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r234129 | tilghman | 2009-12-10 10:24:26 -0600 (Thu, 10 Dec 2009) | 16 lines
 
 Merged revisions 234095 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r234095 | tilghman | 2009-12-10 10:08:20 -0600 (Thu, 10 Dec 2009) | 9 lines
   
   When we receive no response at all to our INVITE, allow the channel to be destroyed.
   (closes issue ASTERISK-14576)
    Reported by: falves11
    Patches:
          20091209__issue15627__1.6.0.diff.txt uploaded by tilghman (license 14)
          20091209__issue15627__1.4.diff.txt uploaded by tilghman (license 14)
    Tested by: falves11
   Review: https://reviewboard.asterisk.org/r/446/
   (closes issue ASTERISK-14653)
   Reported by: dant
   (closes issue ASTERISK-15161)
   Reported by: corruptor
   (closes issue ASTERISK-14337)
   Reported by: falves11
   (issue ASTERISK-14500)
   Reported by: lftsy
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=234133