[Home]

Summary:ASTERISK-15655: [patch] [regression] One-legged Transfer (INVITE / Replaces) not working anymore
Reporter:Philipp Walker (pwalker)Labels:
Date Opened:2010-02-18 07:57:20.000-0600Date Closed:2010-02-23 10:51:25.000-0600
Priority:BlockerRegression?Yes
Status:Closed/CompleteComponents:Channels/chan_sip/Transfers
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) full_replaces_1429_20100218-1.log
( 1) replaces_deadlock_1.4
Description:It seems that the "One legged Transfer" (INVITE with replaces: header) is not working anymore in newer Versions of the 1.4 Branch.
The function has been working up to Asterisk 1.4.26.3, but doesn't starting from 1.4.27.1 (including latest stable 1.4.29 and 1.4.30-rc2). (With exactely the same configuration.)

Scenario:
- 10013 ("Test3") calls 10012  ("Test2")
- 10014 ("Test4") tries to pick up the call
Result:
- On phone 10012, the call is ended
- 10013 seems to answer / pick up the call, but there's no connection (no audio)
- 10014 keeps playing the Ring-back Tone


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

It's more or less guessing, but it could be that one of the changes to fix issue 0015151 (SVN Revision 219303) broke the behaviour.

Note: I'm using a patched Version of Asterisk to allow Call Pickup as implemented e.g. on snom phones (see http://wiki.snom.com/Features/Call_Pick-Up#.281.29_Replaces-_Header ) , that's why the NOTIFYs look different and there are lines saying "Sent call-pickup info to peer x" in the log.
The patch is http://www.net-performer.de/asterisk/asterisk-1.4.22-pickup-by-call-id.patch (modified to apply on Asterisk 1.4.29).
(There might be other Use Cases with an unpatched Asterisk where the "one-legged transfer" is used.)
Comments:By: Philipp Walker (pwalker) 2010-02-18 09:48:28.000-0600

Correction/addition to the results above:
- 10014 seems to answer / pick up the call, but there's no connection (no audio)
- 10013 keeps playing the Ring-back Tone
Extension 10014 stays in "InUse" state, even if all calls were hung up.
By: Philipp Walker (pwalker) 2010-02-19 09:34:20.000-0600

Further investigation with more debugging options enabled:
After performing the above Scenario and terminating all calls, there still are two lock left (core show locks):
=======================================================================
=== Currently Held Locks ==============================================
=======================================================================
===
=== <file> <line num> <function> <lock name> <lock addr> (times locked)
===
=== Thread ID: -1211597936 (do_monitor           started at [17088] chan_sip.c restart_monitor())
=== ---> Lock #0 (chan_sip.c): MUTEX 9695 get_sip_pvt_byid_locked &sip_pvt_ptr->lock 0x834a678 (1)
=== ---> Lock #1 (chan_sip.c): MUTEX 9731 get_sip_pvt_byid_locked (channel lock) 0x8349870 (1)
=== -------------------------------------------------------------------
===
=== Thread ID: -1224025200 (pbx_thread           started at [ 2641] pbx.c ast_pbx_start())
=== ---> Waiting for Lock #0 (channel.c): MUTEX 1745 ast_waitfor_nandfds (channel lock) 0x8349870 (1)
=== --- ---> Locked Here: chan_sip.c line 9731 (get_sip_pvt_byid_locked)
=== -------------------------------------------------------------------
===
=======================================================================
By: Leif Madsen (lmadsen) 2010-02-19 12:39:18.000-0600

Thanks for the well thought out issue report.
By: Philipp Walker (pwalker) 2010-02-19 13:54:40.000-0600

Quick fix to get one legged transfers working:
In channels/chan_sip.c - handle_invite_replaces(), bring back in some of the lines which were removed in SVN Revision 219303 ( http://svnview.digium.com/svn/asterisk?revision=219303&view=revision )
- at the end of the function, where stuff is being unlocked (approx. @ Line 14452) add the line
ast_channel_unlock(p->owner); /* Unlock new owner */
(btw: is "ast_channel_unlock(c);" really correct there?)
- a bit more up in the function, after "Invite/Replaces: preparing to masquerade ..." (approx. Line 14414), add
/* Unlock PVT */
ast_mutex_unlock(&p->refer->refer_call->lock);

WARNING: This might break other things, e.g. especially attended / semi-attended transfers!!! But the one-legger / Pick Up works with no Locks left.
Will have to do more testing, especially on the regular transfer Use Cases.


By: David Vossel (dvossel) 2010-02-22 15:01:30.000-0600

I uploaded a patch.  Please test it and verify it resolves the issue.
By: Philipp Walker (pwalker) 2010-02-23 03:27:22.000-0600

Wow, that was quick! Thanks, David!
Tested with (patched) Asterisk 1.4.29: Pickup Case works, no locks left.
Attention: I've only tested the Pickup scenario, no regular transfers!!!
If you want me to test some other cases too, I need more testing instructions. Attended transfers using snom phones don't seem to go through  handle_invite_replaces().
...and a note on the code: I don't particularly like the "goto request_invite_cleanup;" part of the patch, but it's not much worse than the multiple returns that were there before. And maybe you could fix the typo "replcaes" in "Sending this call to the invite/replcaes handler %s\n" at line 15171? (appears in your patch, but has been there for a long time, I guess...)


By: David Vossel (dvossel) 2010-02-23 09:44:26.000-0600

Thanks for the feedback!  I verified regular transfers work as well.  I'll get this committed shortly.
By: Digium Subversion (svnbot) 2010-02-23 10:26:06.000-0600

Repository: asterisk
Revision: 248396

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r248396 | dvossel | 2010-02-23 10:26:05 -0600 (Tue, 23 Feb 2010) | 9 lines

fixes invite with replaces deadlock

(closes issue ASTERISK-15655)
Reported by: pwalker
Patches:
     replaces_deadlock_1.4 uploaded by dvossel (license 671)
Tested by: pwalker, dvossel


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

http://svn.digium.com/view/asterisk?view=rev&revision=248396
By: Digium Subversion (svnbot) 2010-02-23 10:34:40.000-0600

Repository: asterisk
Revision: 248397

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r248397 | dvossel | 2010-02-23 10:34:39 -0600 (Tue, 23 Feb 2010) | 15 lines

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

........
 r248396 | dvossel | 2010-02-23 10:26:05 -0600 (Tue, 23 Feb 2010) | 9 lines
 
 fixes invite with replaces deadlock
 
 (closes issue ASTERISK-15655)
 Reported by: pwalker
 Patches:
       replaces_deadlock_1.4 uploaded by dvossel (license 671)
 Tested by: pwalker, dvossel
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=248397
By: Digium Subversion (svnbot) 2010-02-23 10:37:49.000-0600

Repository: asterisk
Revision: 248398

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

------------------------------------------------------------------------
r248398 | dvossel | 2010-02-23 10:37:49 -0600 (Tue, 23 Feb 2010) | 22 lines

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

................
 r248397 | dvossel | 2010-02-23 10:34:39 -0600 (Tue, 23 Feb 2010) | 15 lines
 
 Merged revisions 248396 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r248396 | dvossel | 2010-02-23 10:26:05 -0600 (Tue, 23 Feb 2010) | 9 lines
   
   fixes invite with replaces deadlock
   
   (closes issue ASTERISK-15655)
   Reported by: pwalker
   Patches:
         replaces_deadlock_1.4 uploaded by dvossel (license 671)
   Tested by: pwalker, dvossel
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=248398
By: Digium Subversion (svnbot) 2010-02-23 10:48:38.000-0600

Repository: asterisk
Revision: 248399

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

------------------------------------------------------------------------
r248399 | dvossel | 2010-02-23 10:48:37 -0600 (Tue, 23 Feb 2010) | 22 lines

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

................
 r248397 | dvossel | 2010-02-23 10:34:39 -0600 (Tue, 23 Feb 2010) | 15 lines
 
 Merged revisions 248396 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r248396 | dvossel | 2010-02-23 10:26:05 -0600 (Tue, 23 Feb 2010) | 9 lines
   
   fixes invite with replaces deadlock
   
   (closes issue ASTERISK-15655)
   Reported by: pwalker
   Patches:
         replaces_deadlock_1.4 uploaded by dvossel (license 671)
   Tested by: pwalker, dvossel
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=248399
By: Digium Subversion (svnbot) 2010-02-23 10:51:24.000-0600

Repository: asterisk
Revision: 248400

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

------------------------------------------------------------------------
r248400 | dvossel | 2010-02-23 10:51:24 -0600 (Tue, 23 Feb 2010) | 22 lines

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

................
 r248397 | dvossel | 2010-02-23 10:34:39 -0600 (Tue, 23 Feb 2010) | 15 lines
 
 Merged revisions 248396 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r248396 | dvossel | 2010-02-23 10:26:05 -0600 (Tue, 23 Feb 2010) | 9 lines
   
   fixes invite with replaces deadlock
   
   (closes issue ASTERISK-15655)
   Reported by: pwalker
   Patches:
         replaces_deadlock_1.4 uploaded by dvossel (license 671)
   Tested by: pwalker, dvossel
 ........
................

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

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