[Home]

Summary:ASTERISK-15698: [patch] SIP autocreate peers registered when request to unregister
Reporter:Kirill Katsnelson (kkm)Labels:
Date Opened:2010-02-26 00:07:13.000-0600Date Closed:2010-04-15 15:34:45
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Registration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20100412__issue16908.diff.txt
( 1) events.txt
( 2) mgr4b
Description:When autocreate is enabled and a peer is *not* registered, and such a peer sends a REGISTER message with the "Expires: 0" header (to indicate it wants to unregister), the peer is apparently registered and then partially(?) unregistered:
* There is a NOTICE-level option poke message;
* The sequence of PeerStatus manager events shows 3 events:
  1. Unregistered (that's expected)
  2. Registered (that's not)
  3. Reachable



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

1. Enable qualification (not essential for repro but exposes more unexpected behavior)
2. Enable autocreate and register a UAC with a non-existing peer name so autocreate actually happens; I am using the name "nonex" here. I am using Counterpath Brea on Windows, so X-Lite will likely work as well for the repro.
3. do "core restart now"
4. Connect a manager with "read=system" only and login, have it dump received data. A little expect(1) script attached for convenience.
5. set core debug 4 and core verbose 4.
6. You can confirm that the peer "nonex" does not exist in sip show peers.
7. Go to the UAC and unregister it from asterisk.

Attached events.txt lists debug events and manager output. 10.20.0.116 is the Asterisk server and 192.168.0.103 is the client.

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

The scariest thing is that I never yet seen the ast_debug(3,Destroying SIP peer %s) message which comes the first line in sip_destroy_peer(). The peers seem to leak, and every one of them does.

The repro is synthetic. I caught the problem in a much worse situation:
1. UAC unregisters.
2. Asterisk sends 200 OK. The reply packet is lost.
3. UAC repeats in 500 ms. Asterisk knows it's a retry:
chan_sip.c: Ignoring SIP message because of retransmit (REGISTER Seqno 31241, ours 31241)
   but goes on into the registration code. The peer already unlinked by that point, and the scenario occurs as described.

Too bad I do not understand how the autoregistration code is written. It calls temp_peer() before looking at the packet, calls parse_register_contact() then on failure just sends a reply back, never even trying to unlink the peer or anything. Where is the magic by which the peer disappears from the peers list, I do not know; nor can I find why they leak. I am not qualified to fix this code, sorry.
Comments:By: Tilghman Lesher (tilghman) 2010-03-24 11:30:27

Which SVN branch is this?  You've indicated SVN in the version, but without the branch, it's difficult to figure out which one.
By: Kirill Katsnelson (kkm) 2010-03-25 20:50:27

2010-03-11 09:44   lmadsen   Asterisk Version   1.6.2.4 => SVN

I caught the bug in rel. 1.6.2.4.
By: Tilghman Lesher (tilghman) 2010-04-12 15:48:01

I think this patch should fix the situation.  What it does is to check the value of the Expires header (after it detects the presence of a duplicate message) before trying to load the peer into memory.  If it's set to expire the peer and the peer isn't already in memory, then it will just issue a blanket 200 OK and exit without loading the peer.  I believe this should fix the issue you are seeing.
By: Kirill Katsnelson (kkm) 2010-04-12 15:51:29

Thanks, I'll try the patch in a day or two.
By: Kirill Katsnelson (kkm) 2010-04-15 01:22:06

Thank you, the patch totally fixed our problem!

I have been running a production server with the patch for 48 hours now. Without it, we would have seen dozens of failures already (a monitor program depends on registration notifications coming in correct order), and there were none.
By: Digium Subversion (svnbot) 2010-04-15 15:24:52

Repository: asterisk
Revision: 257467

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r257467 | tilghman | 2010-04-15 15:24:51 -0500 (Thu, 15 Apr 2010) | 13 lines

Don't recreate peer, when responding to a repeated deregistration attempt.

When a reply to a deregistration is lost in transmit, the client retries the
deregistration.  Previously, this would cause a realtime/autocreate peer to be
loaded back into memory, after it had already been correctly purged.  Instead,
we just want to resend the reply without loading the peer.

(closes issue ASTERISK-15698)
Reported by: kkm
Patches:
      20100412__issue16908.diff.txt uploaded by tilghman (license 14)
Tested by: kkm

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

http://svn.digium.com/view/asterisk?view=rev&revision=257467
By: Digium Subversion (svnbot) 2010-04-15 15:30:16

Repository: asterisk
Revision: 257493

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r257493 | tilghman | 2010-04-15 15:30:15 -0500 (Thu, 15 Apr 2010) | 20 lines

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

........
 r257467 | tilghman | 2010-04-15 15:24:50 -0500 (Thu, 15 Apr 2010) | 13 lines
 
 Don't recreate peer, when responding to a repeated deregistration attempt.
 
 When a reply to a deregistration is lost in transmit, the client retries the
 deregistration.  Previously, this would cause a realtime/autocreate peer to be
 loaded back into memory, after it had already been correctly purged.  Instead,
 we just want to resend the reply without loading the peer.
 
 (closes issue ASTERISK-15698)
  Reported by: kkm
  Patches:
        20100412__issue16908.diff.txt uploaded by tilghman (license 14)
  Tested by: kkm
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=257493
By: Digium Subversion (svnbot) 2010-04-15 15:34:29

Repository: asterisk
Revision: 257507

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

------------------------------------------------------------------------
r257507 | tilghman | 2010-04-15 15:34:29 -0500 (Thu, 15 Apr 2010) | 27 lines

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

................
 r257493 | tilghman | 2010-04-15 15:30:15 -0500 (Thu, 15 Apr 2010) | 20 lines
 
 Merged revisions 257467 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r257467 | tilghman | 2010-04-15 15:24:50 -0500 (Thu, 15 Apr 2010) | 13 lines
   
   Don't recreate peer, when responding to a repeated deregistration attempt.
   
   When a reply to a deregistration is lost in transmit, the client retries the
   deregistration.  Previously, this would cause a realtime/autocreate peer to be
   loaded back into memory, after it had already been correctly purged.  Instead,
   we just want to resend the reply without loading the peer.
   
   (closes issue ASTERISK-15698)
    Reported by: kkm
    Patches:
          20100412__issue16908.diff.txt uploaded by tilghman (license 14)
    Tested by: kkm
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=257507
By: Digium Subversion (svnbot) 2010-04-15 15:34:37

Repository: asterisk
Revision: 257508

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

------------------------------------------------------------------------
r257508 | tilghman | 2010-04-15 15:34:37 -0500 (Thu, 15 Apr 2010) | 27 lines

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

................
 r257493 | tilghman | 2010-04-15 15:30:15 -0500 (Thu, 15 Apr 2010) | 20 lines
 
 Merged revisions 257467 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r257467 | tilghman | 2010-04-15 15:24:50 -0500 (Thu, 15 Apr 2010) | 13 lines
   
   Don't recreate peer, when responding to a repeated deregistration attempt.
   
   When a reply to a deregistration is lost in transmit, the client retries the
   deregistration.  Previously, this would cause a realtime/autocreate peer to be
   loaded back into memory, after it had already been correctly purged.  Instead,
   we just want to resend the reply without loading the peer.
   
   (closes issue ASTERISK-15698)
    Reported by: kkm
    Patches:
          20100412__issue16908.diff.txt uploaded by tilghman (license 14)
    Tested by: kkm
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=257508
By: Digium Subversion (svnbot) 2010-04-15 15:34:45

Repository: asterisk
Revision: 257510

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

------------------------------------------------------------------------
r257510 | tilghman | 2010-04-15 15:34:44 -0500 (Thu, 15 Apr 2010) | 27 lines

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

................
 r257493 | tilghman | 2010-04-15 15:30:15 -0500 (Thu, 15 Apr 2010) | 20 lines
 
 Merged revisions 257467 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r257467 | tilghman | 2010-04-15 15:24:50 -0500 (Thu, 15 Apr 2010) | 13 lines
   
   Don't recreate peer, when responding to a repeated deregistration attempt.
   
   When a reply to a deregistration is lost in transmit, the client retries the
   deregistration.  Previously, this would cause a realtime/autocreate peer to be
   loaded back into memory, after it had already been correctly purged.  Instead,
   we just want to resend the reply without loading the peer.
   
   (closes issue ASTERISK-15698)
    Reported by: kkm
    Patches:
          20100412__issue16908.diff.txt uploaded by tilghman (license 14)
    Tested by: kkm
 ........
................

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

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