[asterisk-dev] [Code Review] ETSI Explicit Call Transfer (ECT) support. (Asterisk portion)

[rmudgett at digium.com]

> On 2010-04-21 12:48:53, Tilghman Lesher wrote:
> > /trunk/channels/sig_pri.c, lines 1511-1514
> > <https://reviewboard.asterisk.org/r/520/diff/1/?file=8162#file8162line1511>
> >
> >     This series of locks makes me nervous about a possible deadlock.  Could you add a note here that describes why a deadlock is not possible in this situation?  Or alternately, change the code to ensure that a deadlock cannot occur?

Deadlock is not possible here unless the asterisk core needs to do something with the same two channels at the same time.  Is that even possible?

The lock levels involved are: channel, private, and libpri access
Five locks are involved because of the transfer.

Locking from channel to libpri access level:
1) Channel is simply locked.  sig_pri_lock_owner() avoids deadlock here.
2) Private is simply locked.  sig_pri_lock_owner() avoids deadlock here.
3) libpri lock is deadlock avoided by using pri_grab().  pri_grab() does the avoidance dance with the private lock to get the libpri lock.

Locking from libpri access to channel level:
1) libpri access is simply locked and done so by a single thread (pri_dchannel()).  pri_grab() avoids deadlock here.
2) Private is simply locked.  pri_grab() avoids deadlock here.
3) Channel is deadlock avoidance by sig_pri_lock_owner().  sig_pri_lock_owner() does the avoidance dance with the libpri and private locks.

I will reorder the locking to obtain call 1 locks then call 2 locks.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/520/#review1870
-----------------------------------------------------------


On 2010-02-19 15:14:13, rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/520/
> -----------------------------------------------------------
> 
> (Updated 2010-02-19 15:14:13)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Added ability to send and receive ETSI Explicit Call Transfer (ECT) messages to eliminate tromboned calls.
> 
> Asterisk already supported initiating the transfer of calls to eliminate tromboned calls to libpri so there was nothing to do for the asterisk portion.
> 
> The libpri API was extended to indicate to asterisk that the far end requests the transfer of the indicated calls.
> 
> This is a two part review request.  Asterisk and libpri.  This is the asterisk portion.
> https://reviewboard.asterisk.org/r/521/ is the libpri portion.
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 248094 
>   /trunk/channels/chan_dahdi.c 248094 
>   /trunk/channels/sig_pri.h 248094 
>   /trunk/channels/sig_pri.c 248094 
>   /trunk/configs/chan_dahdi.conf.sample 248094 
>   /trunk/configure.ac 248094 
> 
> Diff: https://reviewboard.asterisk.org/r/520/diff
> 
> 
> Testing
> -------
> 
> A -- *1 -- *2  -- B&C
> 
> Party A is on an ISDN phone.
> 
> A calls B
> A puts B on hold
> A calls C
> A hangs up to initiate transfer
> The B and C parties are now connected to each other and the calls are only connected through *2.
> 
> A calls B
> A puts B on hold
> A calls C
> A puts C on hold to retrieve B
> A hangs up to initiate transfer
> The B and C parties are now connected to each other and the calls are only connected through *2.
> 
> Repeat the above two tests but party A uses the EctExecute message instead of disconnecting to transfer the calls.
> 
> All four tests passed.
> 
> 
> Thanks,
> 
> rmudgett
> 
>